Closed
Bug 1210877
Opened 9 years ago
Closed 9 years ago
box-decoration-break: clone and direction: rtl bug
Categories
(Core :: Layout: Block and Inline, defect)
Tracking
()
RESOLVED
FIXED
mozilla44
People
(Reporter: carson, Assigned: MatsPalmgren_bugz)
References
()
Details
(Keywords: css3, rtl, testcase)
Attachments
(2 files)
63.22 KB,
image/png
|
Details | |
8.04 KB,
patch
|
roc
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/45.0.2454.101 Safari/537.36 Steps to reproduce: 1. Create an inline element with a background color, some padding, and box-decoration-break: clone. 2. Add direction: rtl on any other element on the page - it can be anywhere and not related at all 3. The padding on the box-decoration-break element is not set properly. Borders, box shadows, and border radius is. Demo with examples (open in Firefox) - http://jsfiddle.net/cshold/hp1jh7q1/ Actual results: Padding on the box-decoration-break element is not applied, as if box-decoration-break: slice is set but just for padding. Expected results: Padding on the element should be the same as if there were not direction: rtl element on the page.
Comment 1•9 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0 20151002030204
Flags: needinfo?(mats)
Assignee | ||
Comment 2•9 years ago
|
||
Looks like frame size is correct after reflowing the inline but then nsBidiPresUtils::RepositionFrame mess it up. I guess we optimize away the BidiPresUtils stuff when there are no RTL elements so that explains why the extra direction:rtl element is needed to trigger this path.
Assignee: nobody → mats
Flags: needinfo?(mats)
Assignee | ||
Comment 3•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=49a6d6dacc84
Attachment #8669319 -
Flags: review?(roc)
Assignee | ||
Comment 4•9 years ago
|
||
BTW, the code here looks rather weird to me; the comment says "we need to remove any border/padding first", but on line 1595 we're actually *adding* border+padding. And |frameISize| came from aFrame->ISize() on line 1575, which is a *border-box* size. I don't understand how this can work, but apparently it does so I guess the later code compensates for it somehow. Or maybe I'm just misreading it. http://hg.mozilla.org/mozilla-central/annotate/0010c0cb259e/layout/base/nsBidiPresUtils.cpp#l1575 The SetRect() near the end use |rect| which is calculated independently, so perhaps |frameISize| isn't really used in any way that matters. Anyway, my patch is just putting the code that assumes 'slice' under an if-condition that it is.
Attachment #8669319 -
Flags: review?(roc) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/eca3009f91f6
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8669319 [details] [diff] [review] Make nsBidiPresUtils::RepositionFrame work also for box-decoration-break:clone. Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: 'box-decoration-break:clone' doesn't work as expected in some RTL contexts [Describe test coverage new/current, TreeHerder]:we have lot's of 'box-decoration-break' reftests [Risks and why]: low risk, seems worth fixing [String/UUID change made/needed]:none
Attachment #8669319 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•9 years ago
|
Flags: in-testsuite+
Comment 8•9 years ago
|
||
Comment on attachment 8669319 [details] [diff] [review] Make nsBidiPresUtils::RepositionFrame work also for box-decoration-break:clone. Layout fix, has reftests. OK to uplift to aurora.
Attachment #8669319 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 9•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/c63b04aa6e9a
status-firefox43:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•