Currently, in 8.6.x there are 4 instances of @-moz-document url-prefix() {} being used to supply Firefox only css. However, as of June 26th, 2018, with the release of Firefox 61, that rule will no longer be supported. Already, unsupported in dev/nightly/beta, which is why I noticed it since #2886904: display: block for details/summary hides drop arrows in Firefox (normalize.css update) didn't work. The reasons for removal are security/CSS injection/exfiltration.

Three of the instances, are related to a fieldset width Firefox bug that was fixed over 2 years ago. These should probably be removed when #2390621: [policy, no patch] Update Drupal's browser support policy is finally resolved.

Suggested Fix

According to http://browserhacks.com, using @media(min--moz-device-pixel-ratio:0) {} works in all Firefox versions >=4 and matches Drupal's current browser support.

Testing

Testing the change related to #2886904: display: block for details/summary hides drop arrows in Firefox (normalize.css update) is easy:
- Using Firefox 61 (or a beta/dev release since 59)
- Confirm layout.css.moz-document.url-prefix-hack.enabled is disabled in about:config
- Navigate to admin/config/system/site-information
- Confirm that the arrow shows on the summary/details element and toggles on opening/closing the element.

Functional testing the changes related to fieldset width would require a very old Firefox version and I believe it can be skipped as long as a code review shows the changes are equivalent.

CommentFileSizeAuthor
#9 2975388-8-after.png76.91 KBidebr
#9 2975388-8-before.png78.25 KBidebr
#8 2975388-firefox-moz-document-8.patch1.66 KBnickdickinsonwilde
#6 2975388-firefox-moz-document-6.patch1.74 KBnickdickinsonwilde
#2 2975388-firefox-moz-document-2.patch1.88 KBnickdickinsonwilde

Comments

NickWilde created an issue. See original summary.

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Assigned: nickdickinsonwilde » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2975388-firefox-moz-document-2.patch1.88 KB

Patch making changes detailed above.
Ideally, #2886904: display: block for details/summary hides drop arrows in Firefox (normalize.css update) would be cherry picked to 8.5.x and so would this. However, unless #2886904: display: block for details/summary hides drop arrows in Firefox (normalize.css update) is cherrypicked, this does not apply to 8.5.x.

idebr’s picture

idebr commented

I suppose we can just pick any selector hack from http://browserhacks.com?

Personally I find the code be easier to scan if the selector hack takes its own line, for example:

+++ b/core/themes/bartik/css/components/form.css
@@ -23,10 +23,8 @@ fieldset {
-@-moz-document url-prefix() {
-  fieldset {
-    display: table-cell;
-  }

This can be replaced with:

@media screen and (min--moz-device-pixel-ratio:0) {}
  fieldset {
    display: table-cell;
 }
nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented

Yeah any selector hack that works with Firefox 5.x and up would work.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
StatusFileSize
new 2975388-firefox-moz-document-6.patch1.74 KB

Bah! .... https://www.fxsitecompat.com/en-CA/docs/2018/moz-tree-pseudo-elements-ha...
My patch no longer works for Nightly/beta and soon won't work for release branch.
Using a separate line selector hack @idebr if you wanted to re-review that?

idebr’s picture

idebr commented
Status: Needs review » Needs work 
  1. +++ b/core/themes/bartik/css/components/form.css
@@ -23,7 +23,7 @@ fieldset {
+@media(min--moz-device-pixel-ratio:0) { ¶

+++ b/core/themes/seven/css/components/form.css
@@ -19,8 +19,8 @@ fieldset:not(.fieldgroup) {
+@media(min--moz-device-pixel-ratio:0) { ¶

    These lines have a trailing space. 

  2. +++ b/core/themes/seven/css/components/form.css
@@ -41,10 +41,8 @@ fieldset:not(.fieldgroup) > legend {
+_:-moz-tree-row(hover), .fieldgroup {

    This selector was not updated in line with the other Firefox css selectors.

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Status: Needs work » Needs review
StatusFileSize
new 2975388-firefox-moz-document-8.patch1.66 KB

er that's weird.... fixed

idebr’s picture

idebr commented
Title: Firefox support for @-moz-document url-prefix() is being removed » Replace Firefox @-moz-document url-prefix() css selector, since its support is being removed
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 2975388-8-before.png78.25 KB
new 2975388-8-after.png76.91 KB

- Updated the issue summary "Suggested fix" with the new Firefox selector and a reference to browserhacks.com
- Tested the fix in Firefox 62 (stable) with layout.css.moz-document.url-prefix-hack.enabled disabled in about:config, see https://bugzilla.mozilla.org/show_bug.cgi?id=1449753

Before:

After:

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Discussed with @lauriii and we agreed this is a good bugfix and doesn't have any BC concerns.

Committed and pushed 3c15e75d32 to 8.7.x and 996c3f9109 to 8.6.x. Thanks!

core/misc/normalize-fixes.css
 9:6   ✖  Expected single space after at-rule name "@media"                     at-rule-name-space-after
 9:8   ✖  Unexpected unknown media feature name "min--moz-device-pixel-ratio"   media-feature-name-no-unknown
 9:35  ✖  Expected single space after ":"                                       media-feature-colon-space-after


core/themes/bartik/css/components/form.css
 26:6   ✖  Expected single space after at-rule name "@media"                     at-rule-name-space-after
 26:8   ✖  Unexpected unknown media feature name "min--moz-device-pixel-ratio"   media-feature-name-no-unknown
 26:35  ✖  Expected single space after ":"                                       media-feature-colon-space-after


core/themes/seven/css/components/form.css
 22:6   ✖  Expected single space after at-rule name "@media"                     at-rule-name-space-after
 22:8   ✖  Unexpected unknown media feature name "min--moz-device-pixel-ratio"   media-feature-name-no-unknown
 22:35  ✖  Expected single space after ":"                                       media-feature-colon-space-after
 44:6   ✖  Expected single space after at-rule name "@media"                     at-rule-name-space-after
 44:8   ✖  Unexpected unknown media feature name "min--moz-device-pixel-ratio"   media-feature-name-no-unknown
 44:35  ✖  Expected single space after ":"                                       media-feature-colon-space-after

diff --git a/core/.stylelintrc.json b/core/.stylelintrc.json
index d9d0ede9f5..87d1529534 100644
--- a/core/.stylelintrc.json
+++ b/core/.stylelintrc.json
@@ -11,7 +11,10 @@
     "no-duplicate-selectors": null,
     "no-unknown-animations": true,
     "media-feature-name-no-unknown": [true, {
-      "ignoreMediaFeatureNames": ["prefers-reduced-motion"]
+      "ignoreMediaFeatureNames": [
+        "prefers-reduced-motion",
+        "min--moz-device-pixel-ratio"
+      ]
     }],
     "number-leading-zero": "always",
     "plugin/no-browser-hacks": [true, {
diff --git a/core/misc/normalize-fixes.css b/core/misc/normalize-fixes.css
index 2cfdcf311d..b400831192 100644
--- a/core/misc/normalize-fixes.css
+++ b/core/misc/normalize-fixes.css
@@ -6,7 +6,7 @@
 /**
  * Fix problem with details/summary lines missing the drop arrows.
  */
-@media(min--moz-device-pixel-ratio:0) {
+@media (min--moz-device-pixel-ratio: 0) {
   summary {
     display: list-item;
   }
diff --git a/core/themes/bartik/css/components/form.css b/core/themes/bartik/css/components/form.css
index ec75916ab7..e815233fbf 100644
--- a/core/themes/bartik/css/components/form.css
+++ b/core/themes/bartik/css/components/form.css
@@ -23,7 +23,7 @@ fieldset {
  * @todo remove once this Mozilla bug is fixed.
  * See https://bugzilla.mozilla.org/show_bug.cgi?id=504622
  */
-@media(min--moz-device-pixel-ratio:0) {
+@media (min--moz-device-pixel-ratio: 0) {
   fieldset {
     display: table-cell;
   }
diff --git a/core/themes/seven/css/components/form.css b/core/themes/seven/css/components/form.css
index 59399040bd..68dedf1745 100644
--- a/core/themes/seven/css/components/form.css
+++ b/core/themes/seven/css/components/form.css
@@ -19,7 +19,7 @@ fieldset:not(.fieldgroup) {
  * @todo remove once this Mozilla bug is fixed.
  * See https://bugzilla.mozilla.org/show_bug.cgi?id=504622
  */
-@media(min--moz-device-pixel-ratio:0) {
+@media (min--moz-device-pixel-ratio: 0) {
   fieldset:not(.fieldgroup) {
     display: table-cell;
   }
@@ -41,7 +41,7 @@ fieldset:not(.fieldgroup) > legend {
  * @todo remove once this Mozilla bug is fixed.
  * See https://bugzilla.mozilla.org/show_bug.cgi?id=504622
  */
-@media(min--moz-device-pixel-ratio:0) {
+@media (min--moz-device-pixel-ratio: 0) {
   .fieldgroup {
     display: table-cell;
   }

There were some CSS coding style things to fix on commit.

  • alexpott committed 3c15e75 on 8.7.x 
    Issue #2975388 by NickWilde, idebr: Replace Firefox @-moz-document url-...

  • alexpott committed 996c3f9 on 8.6.x 
    Issue #2975388 by NickWilde, idebr: Replace Firefox @-moz-document url-...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.