Closed Bug 1043706 Opened 7 years ago Closed 7 years ago

Error in parsing value for display related to ruby in ua.css

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: khuey, Assigned: sgbowen8)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

When booting up b2g I see a number of "Error in parsing value for 'display'.  Declaration dropped" warnings from ua.css with rules that are display: ruby-something.
In the past we've found ways to comment out or #ifdef out stuff like this, though it's a pain.  I wonder if we should make presence of pref'd-out properties in the UA style sheet not be errors, given that we've run into this problem enough.
(The chunk of ua.css in question was added in bug 1021952 part 2, FWIW.)

I think we can work around this by wrapping this chunk of ua.cs in an appropriate @supports condition.

e.g. "@supports (display:ruby) { ... }"

(I proposed doing this for a similar flexbox issue a while back, in bug 798592, but at that point it wasn't feasible, because (1) @supports wasn't enabled in release builds yet, and (2) we'd still spam the warnings despite the @supports guard (bug 807336). Fortunately, neither of those should be a problem anymore.)
Blocks: 1021952
Ah, indeed, that's a perfect solution.
Assignee: nobody → sgbowen8
Attached patch ua-css-bug.patch (obsolete) — Splinter Review
Here's the fix dholbert suggested. khuey: can you verify that this makes the error messages go away on b2g? They don't show up on desktop in any case.
Attachment #8462882 - Flags: review?(dholbert)
Flags: needinfo?(khuey)
Comment on attachment 8462882 [details] [diff] [review]
ua-css-bug.patch

commit-message nit:
>Bug 1043706 - Avoid spamming ruby display errors in b2g. r=dholbert

Per our "describe-the-change" philosophy of commit messages, the commit message should be more like:

Bug 1043706 - Wrap ruby-specific rules in ua.css in a "@supports" clause (to avoid spamming parse errors when ruby is disabled). r=dholbert

r=me with an updated commit message along those lines.
Attachment #8462882 - Flags: review?(dholbert) → review+
(In reply to Daniel Holbert [:dholbert] from comment #5)
> Per our "describe-the-change" philosophy of commit messages

(reference: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Checkin_comment )
Indeed, that wasn't a great commit message. Thanks for pointing that out.
Attachment #8462882 - Attachment is obsolete: true
Attachment #8462890 - Flags: review?(dholbert)
Comment on attachment 8462890 [details] [diff] [review]
ua-css-bug.patch v2

Looks good - thanks!
Attachment #8462890 - Flags: review?(dholbert) → review+
Keywords: checkin-needed
Version: unspecified → Trunk
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/fc4786cdbbb8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Haven't seen this since.
Flags: needinfo?(khuey)
You need to log in before you can comment on or make changes to this bug.