Print styles don't override default styles properly

This does seem like a random test failure. I queued up drupalci using php 5.6 and php 5.5 pgsql to see if that makes any difference.

I'll spin up my local testbot, and see if I can get any more details about that failure. I need to rebuild all my containers though as I last built them 2 weeks ago.

PHP-5.6 Mysql 5.5 testbot passes. This does seem like a random test failure so I added the Random test failure tag. I am still rebuilding my drupalci containers locally so I can see if it that particular test fails on a testbot in the U.S.

I have a feeling that drupal.org had a hiccup for getting updates.

LocaleUpdateTest has had random test failures before and it used to be quite frequent in #2443633: PostgreSQL: Fix locale\Tests\LocaleUpdateTest, #2157927: Intermittent test fails in LocaleUpdateTest::testUpdateImportSourceRemote(), and more recently #2229979: Random test failure in SimpleXMLElement in Locale tests.

I confirmed that on PHP-5.5-mysql-5.5 testbot that I just rebuilt that the locale tests pass, and I am setting this back to Needs Review.

Ninja edit: I added back the tasks in the issue summary to show what you have completed so far with "(done)" appended to the task, @juhog.

I reviewed the CSS changes for any coding standard regressions, and the patch conforms to CSS changes. Also the patch does what is intended. The two selectors are necessary because the seven theme defines different styles for .button and .button--primary respectively.

All current tests passing. I could not find an existing test that specifically tries to test the DOM for CSS media behavior. CSS Asset grouping already has a unit test that passes.

One thing that still needs to be done is adding a Beta Evaluation into the issue summary so that a core committer can confirm that the category and priority are correctly identified. After that's added, I think this is RTBC unless any front end folks have any specific review comments.

This issue should be reviewed and the summary updated with regard to RC triage.

Scheduled a re-test of #4.

Hello, I am going to evaluate this issue for rc eligibility.

Hello,

Evaluated this for RC eligibility and believe that it should be included. The reasoning for this is that the patch does not change any code, it makes the seven theme consistent with the bartik theme in regards to the location on the print.css.

::Forgot to note that the summary has been updated.::

I think this is not rc eligible because it is not a non-disruptive coding standard change, doc fix, or library update per https://www.drupal.org/core/d8-allowed-changes#rc. This, however, could be a candidate for rc target for the reason that @vwX gave above.

Reran test bot and reviewed patch. Agree with comment #16.

Discussed with @lauriii. Since both Seven and Bartik are going to be treated as internal, this bugfix will potentially be safe to make in a patch release. Since it is not that high-impact, there is not a strong case for it to go in during the RC phase. Thanks!

Also, usually we add the "random test failure" tag to issues that discuss the failure themselves, not just where a random failure result occurs, so removing that tag as well.

I assume this means 8.1

hi @juhog, CSS doesn't get tested, that must be random.

Here's a review to help move this along. Would you mind posting a new patch and we can see how it takes with the test bot (i'll bet it will have no problems)

1. +++ b/core/themes/seven/css/base/print.css
   @@ -42,12 +43,16 @@
   -  .button, .button--primary {
   -    background: none !important;
   +  .button,
   +  .button--primary {
   +    background-image: none;
   +    border-color: #999;
   +    font-weight: inherit;
   

   I'm curious why the font-weight: inherit; is needed? And wouldn't this make background-color show now? or an image with a more specific selector like .js .form-actions .dropbutton-wrapper .dropbutton-widget .dropbutton-action .button I'm guessing you want to keep it because that button has #fff text, or is that overriden someplace else?

2. +++ b/core/themes/seven/css/base/print.css
   @@ -42,12 +43,16 @@
      .messages {
        border-width: 1px;
        border-color: #999;
   +    margin-left: 0;
      }
   

   Why only left is adjusted, wouldn't we need to adjust right too, or at least margin-right for RTL?

Re rolled as #4 failed to apply. And fixed #29.

I couldn't generate an interdiff between #4 or #33 as the tool errors out due to rejected hunks.

Reviewing the the patch in #33 manually I see the following changes:

1. No longer necessary to fix the input selectors as in patch #4 as that is in core already.
2. No longer necessary to fix the button selectors as in patch #4 as that is in core already. The patch in #33 correctly re-rolls this to include the style changes in #4.
3. #29 is resolved by adding margin-right: auto; in patch #33.

I think that the fix for #29 probably needs frontend review for that?

Rerolled patch.

Hi all ,

The patch applies cleanly to 8.5.x. Since I am using Ubuntu I cannot say for W7 or OSx but the patch works as expected. Please find the attachment.

Changing to RTBC as no other review is done.

Rerolled Patch, Kindly someone test and review.

The patch from #36 did/does not require a re-roll - it applies cleanly to 8.6.x and passes the tests.

I am re-uploading a copy of the patch from #36 so that it is the latest patch (and therefore the one run by the testbot).

#46 Patch Failed

#54 Please review the patch

Removing Needs reroll tag as it is no longer needed.

The Seven theme has been removed from Drupal 10 core. I confirmed that this issue only affects Seven and no other themes included with Drupal core, so I am moving this to the contributed Seven project.

Thank you!

Amending attribution.