Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Bug 1487656 ‑ Comment out the indicator :matches(…) block for now #4961

Merged
merged 1 commit into from Sep 13, 2018

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Aug 31, 2018

Fixes bug 1487656:

Inspecting the minified stylesheet, it turns out that the minifier actually merged the :matches(…) and non‑:matches(…) blocks together, which breaks on browsers which don’t support :matches(…).

A temporary solution would be to comment out the :matches(…) block, but a long term one will require a way to tell the CSS minifier to not merge the :matches(…) and non‑:matches(…) blocks together.

.note+.note, .note+.notice, .note+.overheadIndicator, .note+.warning,
.note+p:empty+.note, .note+p:empty+.notice,
.note+p:empty+.overheadIndicator, .note+p:empty+.warning, .note+p:empty+pre,
.note+pre, .notice+.note, .notice+.notice, .notice+.overheadIndicator,
.notice+.warning, .notice+p:empty+.note, .notice+p:empty+.notice,
.notice+p:empty+.overheadIndicator, .notice+p:empty+.warning,
.notice+p:empty+pre, .notice+pre, .overheadIndicator+.note,
.overheadIndicator+.notice, .overheadIndicator+.overheadIndicator,
.overheadIndicator+.warning, .overheadIndicator+p:empty+.note,
.overheadIndicator+p:empty+.notice,
.overheadIndicator+p:empty+.overheadIndicator,
.overheadIndicator+p:empty+.warning, .overheadIndicator+p:empty+pre,
.overheadIndicator+pre, .warning+.note, .warning+.notice,
.warning+.overheadIndicator, .warning+.warning, .warning+p:empty+.note,
.warning+p:empty+.notice, .warning+p:empty+.overheadIndicator,
.warning+p:empty+.warning, .warning+p:empty+pre, .warning+pre,
:matches(.warning, .overheadIndicator, .note, .notice,
pre)+:matches(.warning, .overheadIndicator, .note, .notice, pre),
:matches(.warning, .overheadIndicator, .note, .notice,
pre)+p:empty+:matches(.warning, .overheadIndicator, .note, .notice, pre),
pre+.note, pre+.notice, pre+.overheadIndicator, pre+.warning,
pre+p:empty+.note, pre+p:empty+.notice, pre+p:empty+.overheadIndicator,
pre+p:empty+.warning, pre+p:empty+pre, pre+pre {margin-top:-15px}

Follow‑up to #4958, necessary for #4884


review?(@jwhitlock)

See also w3c/csswg-drafts#3082, which is the reason why we are in this situation to begin with.

@ExE-Boss ExE-Boss force-pushed the styles/wiki/indicators/fix-snugging branch 2 times, most recently from b6b6994 to a4e1ba8 Compare August 31, 2018 11:33
Copy link
Contributor

@jwhitlock jwhitlock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put my local dev environment into production assets mode. Before this change, the minimized rule is:

.note+.note,.note+.notice,.note+.overheadIndicator,.note+.warning,.note+p:empty+.note,.note+p:empty+.notice,.note+p:empty+.overheadIndicator,.note+p:empty+.warning,.note+p:empty+pre,.note+pre,.notice+.note,.notice+.notice,.notice+.overheadIndicator,.notice+.warning,.notice+p:empty+.note,.notice+p:empty+.notice,.notice+p:empty+.overheadIndicator,.notice+p:empty+.warning,.notice+p:empty+pre,.notice+pre,.overheadIndicator+.note,.overheadIndicator+.notice,.overheadIndicator+.overheadIndicator,.overheadIndicator+.warning,.overheadIndicator+p:empty+.note,.overheadIndicator+p:empty+.notice,.overheadIndicator+p:empty+.overheadIndicator,.overheadIndicator+p:empty+.warning,.overheadIndicator+p:empty+pre,.overheadIndicator+pre,.warning+.note,.warning+.notice,.warning+.overheadIndicator,.warning+.warning,.warning+p:empty+.note,.warning+p:empty+.notice,.warning+p:empty+.overheadIndicator,.warning+p:empty+.warning,.warning+p:empty+pre,.warning+pre,:matches(.warning,.overheadIndicator,.note,.notice,pre)+:matches(.warning,.overheadIndicator,.note,.notice,pre),:matches(.warning,.overheadIndicator,.note,.notice,pre)+p:empty+:matches(.warning,.overheadIndicator,.note,.notice,pre),pre+.note,pre+.notice,pre+.overheadIndicator,pre+.warning,pre+p:empty+.note,pre+p:empty+.notice,pre+p:empty+.overheadIndicator,pre+p:empty+.warning,pre+p:empty+pre,pre+pre {
 margin-top:-15px
}

The indicators have the desired spacing in Safari, but not in Chrome or Firefox:

pr 4961 - before

After this PR, the rule is:

.note+.note,.note+.notice,.note+.overheadIndicator,.note+.warning,.note+p:empty+.note,.note+p:empty+.notice,.note+p:empty+.overheadIndicator,.note+p:empty+.warning,.note+p:empty+pre,.note+pre,.notice+.note,.notice+.notice,.notice+.overheadIndicator,.notice+.warning,.notice+p:empty+.note,.notice+p:empty+.notice,.notice+p:empty+.overheadIndicator,.notice+p:empty+.warning,.notice+p:empty+pre,.notice+pre,.overheadIndicator+.note,.overheadIndicator+.notice,.overheadIndicator+.overheadIndicator,.overheadIndicator+.warning,.overheadIndicator+p:empty+.note,.overheadIndicator+p:empty+.notice,.overheadIndicator+p:empty+.overheadIndicator,.overheadIndicator+p:empty+.warning,.overheadIndicator+p:empty+pre,.overheadIndicator+pre,.warning+.note,.warning+.notice,.warning+.overheadIndicator,.warning+.warning,.warning+p:empty+.note,.warning+p:empty+.notice,.warning+p:empty+.overheadIndicator,.warning+p:empty+.warning,.warning+p:empty+pre,.warning+pre,pre+.note,pre+.notice,pre+.overheadIndicator,pre+.warning,pre+p:empty+.note,pre+p:empty+.notice,pre+p:empty+.overheadIndicator,pre+p:empty+.warning,pre+p:empty+pre,pre+pre {
 margin-top:-15px
}

The indicators are now snug in Safari, Chrome, and Firefox:

pr 4961 - after

@jwhitlock jwhitlock merged commit fb753b0 into mdn:master Sep 13, 2018
@ExE-Boss ExE-Boss deleted the styles/wiki/indicators/fix-snugging branch September 13, 2018 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants