Closed Bug 1365629 Opened 6 years ago Closed 6 years ago

stylo: Figure out why a bunch of SVG reftests were passing before bug 1364746 landed.

Categories

(Core :: CSS Parsing and Computation, enhancement)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1365629: minor cleanup in ServoCSSParsingEnvironment code.
59 bytes, text/x-review-board-request
canova
: review+
Details 
Read https://bugzilla.mozilla.org/show_bug.cgi?id=1364746#c11 for fun.

The gecko side of that patch hadn't landed, so we were passing presumably a random register as the quirks mode.

That made a lot of reftests that expected unitless lengths to be parsed to pass, consistently.

So something is probably bogus in our handling of quirks mode and ParsingMode.
Flags: needinfo?(canaltinova)
Though I might not quite follow this bug, it's due to bug 1363574, if you are referring unitless length?
(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> Though I might not quite follow this bug, it's due to bug 1363574, if you
> are referring unitless length?

Nope! Unitless lengths are also affected by quirks AIUI, which is what bug 1364746 was about.

The intermediate state of that bug made us pass a bunch of tests, see https://hg.mozilla.org/integration/autoland/rev/0be21772f2f9
04:24 <emilio> canaltinova: so looking at it real quick it looks like it's from SVG attribute parsing code
04:25 <emilio> canaltinova: so I'd look into how the SMIL declaration block is constructed
04:26 <emilio> canaltinova: heh, I think I got it
04:28 <canaltinova> emilio: what, how?
04:28 <emilio> canaltinova: So in nsDOMCSSAttributeDeclaration::GetServoCSSParsingEnvironment(), we don't do anything special if the declaration is a smil override
04:28 <emilio> canaltinova: we should probably give that a ParsingMode::SVG
04:29 <emilio> canaltinova: I need to build it, and I'll probably fall asleep before that happens though
04:29 <emilio> canaltinova: I'll leave a comment on the bug
Flags: needinfo?(emilio+bugs)
Assignee: nobody → emilio+bugs
Flags: needinfo?(emilio+bugs)
Flags: needinfo?(canaltinova)
(In reply to Emilio Cobos Álvarez [:emilio] from comment #3)
> 04:24 <emilio> canaltinova: so looking at it real quick it looks like it's
> from SVG attribute parsing code
> 04:25 <emilio> canaltinova: so I'd look into how the SMIL declaration block
> is constructed
> 04:26 <emilio> canaltinova: heh, I think I got it
> 04:28 <canaltinova> emilio: what, how?
> 04:28 <emilio> canaltinova: So in
> nsDOMCSSAttributeDeclaration::GetServoCSSParsingEnvironment(), we don't do
> anything special if the declaration is a smil override
> 04:28 <emilio> canaltinova: we should probably give that a ParsingMode::SVG
> 04:29 <emilio> canaltinova: I need to build it, and I'll probably fall
> asleep before that happens though
> 04:29 <emilio> canaltinova: I'll leave a comment on the bug

I was completely wrong here btw :)
Comment on attachment 8869819 [details]
Bug 1365629: minor cleanup in ServoCSSParsingEnvironment code.

https://reviewboard.mozilla.org/r/141368/#review144910
Attachment #8869819 - Flags: review?(canaltinova) → review+
Attachment #8869817 - Attachment is obsolete: true
Attachment #8869817 - Flags: review?(hikezoe)
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f76dd202aa38
minor cleanup in ServoCSSParsingEnvironment code. r=canaltinova
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/33be00ba2f75
Update reftest expectations. r=me
You need to log in before you can comment on or make changes to this bug.