Closed Bug 1097128 Opened 5 years ago Closed 5 years ago
turn on the compile-time switch for vertical writing-mode support
We have both a compile-time switch in WritingModes.h that completely disables support for vertical writing modes, and a runtime pref that determines whether the CSS writing-mode, text-orientation, and text-combine-upright properties are actually recognized. At this point, I think we should consider landing the patch to enable the compile-time switch on Nightly (we might want to keep it disabled on release for now). As long as the runtime vertical-text pref remains off, this should have no impact on behavior, but it becomes possible to start testing vertical modes in standard nightly builds by just toggling a pref. This will mean that key low-level mozilla::WritingMode methods such as IsVertical() will actually be testing something, rather than returning a hard-coded value, and so we'll need to watch for any performance impact; the compiler will no longer be able to optimize away various tests, conversion codepaths, etc., throughout Layout. I've tried pushing such a patch to tryserver recently, and looking at the Tp numbers; I couldn't see any consistent difference compared to a standard mozilla-central build, but we still need to be alert for any significant regression.  See https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ebbc87d99221. That push also enabled the runtime vertical-text pref, which currently results in the unit test oranges seen there; the compile-time switch alone will not cause this.
WDYT - are we far enough along that it's worth throwing the switch here, both to check for any perf impact and to make it easier for people to start experimenting?
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Oops, sorry for the noise - I forgot to make it conditional on #ifndef RELEASE_BUILD. For now.
Attachment #8520738 - Flags: review?(smontagu)
Comment on attachment 8520738 [details] [diff] [review] Enable compile-time support for vertical writing modes. Review of attachment 8520738 [details] [diff] [review]: ----------------------------------------------------------------- It is time.
Attachment #8520738 - Flags: review?(smontagu) → review+
OK, let's see how this goes... I'm hopeful this will stick without problems, but we do need to be alert for possible regressions. https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c720eea5b1
Target Milestone: --- → mozilla36
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.