Remove layout.css.text-emphasis.enabled pref

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks 1 bug, {dev-doc-complete})

unspecified
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

4 years ago
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.
Assignee

Comment 1

4 years ago
dbaron, what do you think?
Flags: needinfo?(dbaron)
Assignee

Updated

4 years ago
Depends on: 1229739
Assignee

Comment 2

4 years ago
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.
Assignee

Comment 3

4 years ago
Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.
Attachment #8696528 - Flags: review?(dbaron)
Assignee

Updated

4 years ago
Flags: needinfo?(dbaron)
Assignee

Updated

4 years ago
Depends on: 1231485
Assignee

Comment 4

4 years ago
dbaron, could you review this super simple patch?
Flags: needinfo?(dbaron)
Assignee

Comment 5

4 years ago
As we've missed Firefox 45 for enabling text-emphasis, let's wait for the next version to remove the pref.
Flags: needinfo?(dbaron)
Assignee

Updated

4 years ago
Attachment #8696528 - Flags: review?(dbaron)
Assignee

Comment 7

3 years ago
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

Updated

3 years ago
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.
Assignee

Comment 9

3 years ago
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

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/03bd76ec05cb
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.