Closed Bug 1229609 Opened 5 years ago Closed 5 years ago

Remove layout.css.text-emphasis.enabled pref

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

I don't think we need to follow the general "enable the feature by default" then long later "remove the pref" path, because:
1. this feature is not that complicated so I don't think it would cause any major breakage which would require us to switch it off again;
2. the spec around this feature has been quite stable for a while: some other vendors have been shipping a compitable prefixed version of this feature, and I don't see any major blocker during implementing it.

Given this, I believe we can simply remove the pref to enable it directly.
dbaron, what do you think?
Flags: needinfo?(dbaron)
Depends on: 1229739
One correction: Blink is shipping a mostly compatible prefixed version (whose syntax of text-emphasis-position doesn't match the current spec), and WebKit is shipping a compatible unprefixed version.

Those impls haven't yet had the proper behavior for ruby with emphasis marks, though.
Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.
Attachment #8696528 - Flags: review?(dbaron)
Flags: needinfo?(dbaron)
Depends on: 1231485
dbaron, could you review this super simple patch?
Flags: needinfo?(dbaron)
As we've missed Firefox 45 for enabling text-emphasis, let's wait for the next version to remove the pref.
Flags: needinfo?(dbaron)
Attachment #8696528 - Flags: review?(dbaron)
Comment on attachment 8696528 [details]
MozReview Request: Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27359/diff/1-2/
Attachment #8696528 - Flags: review?(cam)
Assignee: nobody → quanxunzhen
Attachment #8696528 - Flags: review?(cam)
Comment on attachment 8696528 [details]
MozReview Request: Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.

https://reviewboard.mozilla.org/r/27359/#review41941

I think we should wait at least one cycle with the pref enabled on Release (46) before removing the pref.
Comment on attachment 8696528 [details]
MozReview Request: Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27359/diff/2-3/
Attachment #8696528 - Flags: review?(cam)
Attachment #8696528 - Flags: review?(cam) → review+
Comment on attachment 8696528 [details]
MozReview Request: Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.

https://reviewboard.mozilla.org/r/27359/#review52798
https://hg.mozilla.org/mozilla-central/rev/03bd76ec05cb
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.