Teach nsBidiPresUtils to only manipulate continuations created by bidi resolution
Categories
(Core :: Layout: Text and Fonts, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox69 | --- | fixed |
People
(Reporter: TYLin, Assigned: TYLin)
References
Details
Attachments
(2 files)
File per dbaron's comment in https://phabricator.services.mozilla.com/D17551#466846,
Though in the longer term, I think it would probably make more sense for this code to know which continuations were created as a result of bidi resolution and only mess with those; that gives us more flexibility to use fixed continuations for things other than bidi. This code is essentially assuming that bidi is the only user of fixed continuations, and I don't think that's fair. You're fixing it by keeping things separated, but not really fixing what I see as the underlying problem.
The background story is that column-span:all splits nsColumnSetFrames into several fragments, and they're linked together by non-fluid (fixed) continuations, which previously are only created by the results of bidi resolutions. So bidi utils aren't aware of that, and mess with them.
My patches in Bug 1520722 are regards as short term workaround. Depend on the out come of the solution in this bug, we may be able to revert bug 1520722 without any regression.
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 1•5 years ago
|
||
This reverts the modification to nsBidiPresUtils.cpp in Bug 1520722 Part
2, but keeps the test added. Next part will fix the problem in a proper
way.
|
|
Assignee | |
Comment 2•5 years ago
|
||
The idea is to check IsBidiSplittable() in more places to prevent fixed
continuations created by column-span from becoming fluid ones.
Depends on D34092
|
|
Assignee | |
Updated•5 years ago
|
|
||
Comment 3•5 years ago
|
||
Does this also fix the testcase from bug 1554824, by any chance?
|
|
Assignee | |
Comment 4•5 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #3)
Does this also fix the testcase from bug 1554824, by any chance?
Sadly, no. It's a different issue. I'm still seeking a solution to the bug.
|
|
||
Comment 5•5 years ago
|
||
OK, thanks for checking.
Pushed by aethanyc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/79d5b3103bff Part 1 - Revert Bug 1520722 Part 2. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/6f0a648c3d83 Part 2 - Check IsBidiSplittable() before processing the ancestor frames. r=jfkthame
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17275 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/17275 * Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/VD5Nobz7RYKAtbJ-9sOBRA)
|
||
Comment 9•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/79d5b3103bff
https://hg.mozilla.org/mozilla-central/rev/6f0a648c3d83
Upstream PR was closed without merging
Upstream PR merged
Description
•