Closed Bug 1210877 Opened 4 years ago Closed 4 years ago
box-decoration-break: clone and direction: rtl bug
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.
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0 20151002030204
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
Attachment #8669319 - Flags: review?(roc)
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+
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?
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+
You need to log in before you can comment on or make changes to this bug.