Gecko and Stylo serialize counter-{increment,reset} differently

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 1 bug)

53 Branch
mozilla55
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [stylo])

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
When the <integer> part lacks, the serialization from Gecko also lacks that part, while Stylo always serializes the <integer> part with the corresponding default value.

Stylo's behavior currently matches Blink, while Gecko's matches Edge. I don't have a strong opinion on which is correct, and the spec doesn't seem to indicate either way clearly.

Looking at the code, I have a feeling that changing the Gecko parser and test might be easier than changing Servo behavior.

As far as I see, test_bug829816.html is one test which relies on this.
(Assignee)

Updated

2 years ago
Blocks: 1243581
Should we move the failure expectations into the "need Gecko change" section of stylo-failures.md?
(Assignee)

Comment 3

2 years ago
Does that matter? I guess I can just fix this and remove the corresponding item...
Assignee: nobody → xidorn+moz
(Assignee)

Comment 4

2 years ago
In case I forget...
Flags: needinfo?(xidorn+moz)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Flags: needinfo?(xidorn+moz)
(Assignee)

Updated

2 years ago
Attachment #8870255 - Flags: review?(cam)
Comment hidden (mozreview-request)
The current spec https://drafts.csswg.org/css-lists-3/#counter-properties does say "Computed value: specified value", and without explicit serialization guidelines, usually that's what the serialization is based on.  So I think this is the right change.
(In reply to Cameron McCormack (:heycam) from comment #7)
> So I think this is the right change.

I mean: I think we should be changing the other direction...
Safari omits the integer part neither in specified nor in computed values, so we should at least stay consistent between specified and computed.

Gecko is currently omits explicit integers in specified but not in computed.

Let's go ahead with that patch. :)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8870255 [details]
Bug 1363968 - Change how counter-{reset,increment} is parsed to align serialization of specified value with Servo and Blink.

https://reviewboard.mozilla.org/r/141706/#review145374

OK, I guess it's not super important which way we align here.

::: commit-message-c7fdf:1
(Diff revision 2)
> +Bug 1363968 - Change how counter-{reset,incremenet} is parsed to align serialization of specified value with servo and blink. r?heycam

Nit: "increment".

(And, to be even more picky, "Servo" and "Blink".)
Attachment #8870255 - Flags: review?(cam) → review+
Comment hidden (mozreview-request)

Comment 12

2 years ago
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1507721df493
Change how counter-{reset,increment} is parsed to align serialization of specified value with Servo and Blink. r=heycam

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1507721df493
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
https://hg.mozilla.org/projects/cedar/rev/1507721df493580f035695ad088890b5cbb78ba6
Bug 1363968 - Change how counter-{reset,increment} is parsed to align serialization of specified value with Servo and Blink. r=heycam
You need to log in before you can comment on or make changes to this bug.