Closed Bug 1375374 Opened 2 years ago Closed 2 years ago

stylo: Match Gecko's handling of internal properties / prefed properties

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: xidorn, Assigned: heycam)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In Gecko, there is a ENABLED_IN_UA_SHEETS flag for properties, combined with pref, there are four independent different states:
1. no pref, no flag: usable everywhere
2. no pref, has flag: usable in UA sheets, not usable in content sheets
3. has pref, no flag: the pref controls it everywhere
4. has pref, has flag: usable in UA sheets, the pref controls it in content sheets

(Actually we have a ENABLED_IN_UA_SHEETS_AND_CHROME which makes the property usable in chrome documents in addition to UA sheets.)

While in Stylo, we have an internal flag, which is checked independently than pref, and the relationship is:
1. no pref, no flag: usable everywhere
2. no pref, has flag: usable in UA sheets, not usable in content sheets
3. has pref, no flag: the pref controls it everywhere
4. has pref, has flag: the pref controls it in UA sheet, not usable in content


So there are two problems:
The main problem is that, in the last case, the behavior is different between Gecko and Stylo, which can cause some issues.
The other problem is that, because of the difference, we never mark properties which may be usable in content with "internal", which means there are properties which should go to (4) ends up being (2).
This at least affects bug966992-{1,2,3}.html in test_reftests_with_caret.html because of overflow-clip-box.
This turns out to be the root cause of bug 1365822.
Blocks: 1365822
One tricky thing is that, if we put the "enabled_in_ua_sheet" flag to property definitions in Servo code, once we remove a pref without removing the flag at the same time, the state of the property would suddenly jump from (4) to (2), which is unfortunate.

It is fine in Gecko because both pref and the flag are written in nsCSSPropList.h, so it is easier to remove them at the same time. But if they are put in separate places like in Stylo, it would be a bigger problem.

Tests would hopefully catch such change, though... just a bit painful for developer.
Manish, any suggestion what should we do here?
Flags: needinfo?(manishearth)
We can have the pref/flag duplicated in the templates and autogenerate tests that ensure it matches up with gecko.
Flags: needinfo?(manishearth)
I'll take a look at this.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8882062 - Flags: review?(manishearth)
Comment on attachment 8882062 [details]
Bug 1375374 - Test expectation adjustments.

https://reviewboard.mozilla.org/r/153154/#review158452
Attachment #8882062 - Flags: review?(manishearth) → review+
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c38bf53df24b
Test expectation adjustments. r=manishearth
https://hg.mozilla.org/mozilla-central/rev/c38bf53df24b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.