Xidorn Quan [:xidorn] UTC+11 Assignee
	Description • 9 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.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 1 • 9 years ago

dbaron, what do you think?

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 2 • 9 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.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 3 • 9 years ago

Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 4 • 9 years ago

dbaron, could you review this super simple patch?

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 5 • 9 years ago

As we've missed Firefox 45 for enabling text-emphasis, let's wait for the next version to remove the pref.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 6 • 8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d22bebea625

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 7 • 8 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/

	Cameron McCormack (:heycam)
	Comment 8 • 8 years ago

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.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 9 • 8 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/

	Cameron McCormack (:heycam)
	Comment 10 • 8 years ago

Comment on attachment 8696528 [details]
MozReview Request: Bug 1229609 - Remove layout.css.text-emphasis.enabled pref.

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

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 11 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/03bd76ec05cb9b0765e79ab5077a61a533b40168
Bug 1229609 - Remove layout.css.text-emphasis.enabled pref. r=heycam

	Carsten Book [:Tomcat]
	Comment 12 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/03bd76ec05cb

	Jean-Yves Perrier [:teoli]
	Comment 13 • 8 years ago

I added a comment in:
https://developer.mozilla.org/en-US/docs/Web/CSS/text-emphasis#Browser_compatibility
and in:
https://developer.mozilla.org/en-US/Firefox/Releases/49#CSS