Closed Bug 1363968 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Version:
53 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

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

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

(Whiteboard: [stylo])

Attachments

(1 file)

Bug 1363968 - Change how counter-{reset,increment} is parsed to align serialization of specified value with Servo and Blink.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
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.
Blocks: stylo
Should we move the failure expectations into the "need Gecko change" section of stylo-failures.md?
Does that matter? I guess I can just fix this and remove the corresponding item...
Assignee: nobody → xidorn+moz
In case I forget...
Flags: needinfo?(xidorn+moz)
Flags: needinfo?(xidorn+moz)
Attachment #8870255 - Flags: review?(cam)
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/1507721df493
Status: NEW → RESOLVED
Closed: 6 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.