Closed
Bug 1097128
Opened 10 years ago
Closed 10 years ago
turn on the compile-time switch for vertical writing-mode support
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jfkthame, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
1.59 KB,
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
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;[1] 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. [1] 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.
Assignee | ||
Comment 1•10 years ago
|
||
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?
Attachment #8520736 -
Flags: review?(smontagu)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
Oops, sorry for the noise - I forgot to make it conditional on #ifndef RELEASE_BUILD. For now.
Attachment #8520738 -
Flags: review?(smontagu)
Assignee | ||
Updated•10 years ago
|
Attachment #8520736 -
Attachment is obsolete: true
Attachment #8520736 -
Flags: review?(smontagu)
Assignee | ||
Updated•10 years ago
|
Blocks: enable-writing-mode-dev
Comment 3•10 years ago
|
||
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+
Assignee | ||
Comment 4•10 years ago
|
||
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
https://hg.mozilla.org/mozilla-central/rev/c9c720eea5b1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•