1372488 - Make counter style 'disc' not overridable

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Xidorn Quan [:xidorn] UTC+10]

As I proposed in w3c/csswg-drafts#1521. This would allow us to avoid building counter style for initial struct. That would simplify our handling of counter style in stylo.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1372488 - Make counter style 'disc' not overridable.

Review commit: https://reviewboard.mozilla.org/r/148406/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/148406/

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5068316e6040fe6a3e765264dde6d3146ea3ff26
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd61162b6069d211cc19621becbf42006d49e18a

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Also I suspect that, before this change, CounterStyleManager::IsInitial() actually never returns true after the first restyle, because creating nsStyleList would always insert the "disc" item into the table. That said, doing this should allow us to actually benefit from skipping flushing counter style manager in many pages.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think this is a reasonable change to make.  Not allowing @counter-style rules to change the meaning of the initial value of list-style-type seems like a good restriction to add.

The one thing I'm confused about is layout/reftests/counter-style/redefine-attr-mapping-ref.html.  Why does the reference have triangles rather than discs?

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

The changes to redefine-attr-mapping-ref.html correspond to changes in redefine-attr-mapping.html, where I just removed all instances which may lead to using 'disc'.

That test is for checking the given predefined counter styles are indeed overridden. Since 'disc' is no longer overridable, I just remove them from it.

(Not sure what your question really is...)

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8877063 [details]
Bug 1372488 - Make counter style 'disc' not overridable.

https://reviewboard.mozilla.org/r/148406/#review154356

Oops, for some reason I was thinking those lines were added rather than removed.  r=dbaron

Comment 7

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 9 changes to 9 files
remote: (dc243c10abeb modifies servo/components/style/stylesheets/rule_parser.rs from Servo; not enforcing peer review)
remote: (1 changesets contain changes to protected servo/ directory: dc243c10abeb)
remote: ************************************************************************
remote: you do not have permissions to modify files under servo/
remote: 
remote: the servo/ directory is kept in sync with the canonical upstream
remote: repository at https://github.com/servo/servo
remote: 
remote: changes to servo/ are only allowed by the syncing tool and by sheriffs
remote: performing cross-repository "merges"
remote: 
remote: to make changes to servo/, submit a Pull Request against the servo/servo
remote: GitHub project
remote: ************************************************************************
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.e_prevent_vendored hook failed
abort: push failed on remote

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

Servo PR: servo/servo#17353

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8877063 [details]
Bug 1372488 - Make counter style 'disc' not overridable.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/148406/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2f80edad993
Make counter style 'disc' not overridable. r=dbaron

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e2f80edad993