Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 758073003: Always propagate direction and writing-mode from <body>. (Closed)

Created:
6 years ago by andersr
Modified:
6 years ago
Reviewers:
rune
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, ed+blinkwatch_opera.com, rwlbuis, sof
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Always propagate direction and writing-mode from <body>. Even though the specification says that direction/writing-mode are *not* to be propagated from <body>, all major browsers currently do this. In Blink, when opposing direction/writing-modes are explicitly set on both the root element and <body>, the direction/writing-mode of the root element is propagated to the view, ignoring what is specified on <body>. This is different from Firefox and IE, where the direction/writing-mode is always propagated from the computed style of <body>. This patch aligns Blink's behavior to that of Firefox and IE, such that we always propagate from <body>. R=rune@opera.com Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186115

Patch Set 1 #

Patch Set 2 : Fix propagation tests. #

Total comments: 4

Patch Set 3 : Added XML and overconstraining tests. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+166 lines, -63 lines) Patch
A LayoutTests/fast/writing-mode/absolute-overconstrained-direction.html View 1 2 1 chunk +18 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/absolute-overconstrained-direction-expected.html View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/absolute-overconstrained-writing-mode.html View 1 2 1 chunk +18 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/absolute-overconstrained-writing-mode-expected.html View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
M LayoutTests/fast/writing-mode/body-direction-propagation.html View 1 1 chunk +3 lines, -2 lines 0 comments Download
D LayoutTests/fast/writing-mode/body-direction-propagation-blocked.html View 1 1 chunk +0 lines, -3 lines 0 comments Download
D LayoutTests/fast/writing-mode/body-direction-propagation-blocked-expected.png View 1 Binary file 0 comments Download
D LayoutTests/fast/writing-mode/body-direction-propagation-blocked-expected.txt View 1 1 chunk +0 lines, -5 lines 0 comments Download
A LayoutTests/fast/writing-mode/body-direction-propagation-expected.html View 1 1 chunk +5 lines, -0 lines 0 comments Download
D LayoutTests/fast/writing-mode/body-direction-propagation-expected.png View 1 Binary file 0 comments Download
D LayoutTests/fast/writing-mode/body-direction-propagation-expected.txt View 1 1 chunk +0 lines, -5 lines 0 comments Download
M LayoutTests/fast/writing-mode/body-writing-mode-propagation.html View 1 1 chunk +3 lines, -2 lines 0 comments Download
D LayoutTests/fast/writing-mode/body-writing-mode-propagation-blocked.html View 1 1 chunk +0 lines, -3 lines 0 comments Download
D LayoutTests/fast/writing-mode/body-writing-mode-propagation-blocked-expected.png View 1 Binary file 0 comments Download
D LayoutTests/fast/writing-mode/body-writing-mode-propagation-blocked-expected.txt View 1 1 chunk +0 lines, -5 lines 0 comments Download
A LayoutTests/fast/writing-mode/body-writing-mode-propagation-expected.html View 1 1 chunk +5 lines, -0 lines 0 comments Download
D LayoutTests/fast/writing-mode/body-writing-mode-propagation-expected.png View 1 Binary file 0 comments Download
D LayoutTests/fast/writing-mode/body-writing-mode-propagation-expected.txt View 1 1 chunk +0 lines, -5 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-and-body-direction-propagation.html View 1 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-and-body-direction-propagation-expected.html View 1 1 chunk +5 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-and-body-writing-mode-propagation.html View 1 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-and-body-writing-mode-propagation-expected.html View 1 1 chunk +5 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-direction-propagation.html View 1 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-direction-propagation-expected.html View 1 1 chunk +5 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-writing-mode-propagation.html View 1 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/html-writing-mode-propagation-expected.html View 1 1 chunk +5 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/xhtml-no-body-propagation-direction.xml View 1 2 1 chunk +14 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/xhtml-no-body-propagation-direction-expected.xml View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/xhtml-no-body-propagation-writing-mode.xml View 1 2 1 chunk +14 lines, -0 lines 0 comments Download
A LayoutTests/fast/writing-mode/xhtml-no-body-propagation-writing-mode-expected.xml View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
M Source/core/css/resolver/MatchedPropertiesCache.cpp View 1 chunk +0 lines, -3 lines 0 comments Download
M Source/core/css/resolver/StyleBuilderCustom.cpp View 2 chunks +0 lines, -6 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 4 chunks +0 lines, -11 lines 0 comments Download
M Source/core/dom/Document.h View 2 chunks +0 lines, -7 lines 0 comments Download
M Source/core/dom/Document.cpp View 2 chunks +2 lines, -6 lines 0 comments Download

Messages

Total messages: 17 (1 generated)
andersr
6 years ago (2014-11-25 16:44:02 UTC) #1
andersr
This is the origin of the propagation: https://bugs.webkit.org/show_bug.cgi?id=48157 The author tried to get the body ...
6 years ago (2014-11-25 17:15:25 UTC) #2
rune
I agree with aligning with the other browsers. You need to write some test-cases, or ...
6 years ago (2014-11-25 21:41:28 UTC) #3
andersr
On 2014/11/25 21:41:28, rune wrote: > I agree with aligning with the other browsers. You ...
6 years ago (2014-11-26 16:17:53 UTC) #4
andersr
On 2014/11/25 21:41:28, rune wrote: > I agree with aligning with the other browsers. You ...
6 years ago (2014-11-26 16:17:54 UTC) #5
rune
Could you add tests for the over-constrained absolute positioned with initial containing block as well?
6 years ago (2014-11-27 09:51:09 UTC) #6
rune
https://codereview.chromium.org/758073003/diff/20001/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/758073003/diff/20001/Source/core/dom/Document.cpp#newcode1688 Source/core/dom/Document.cpp:1688: rootDirection = bodyStyle->direction(); What if you don't have body? ...
6 years ago (2014-11-27 09:55:04 UTC) #7
andersr
https://codereview.chromium.org/758073003/diff/20001/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/758073003/diff/20001/Source/core/dom/Document.cpp#newcode1688 Source/core/dom/Document.cpp:1688: rootDirection = bodyStyle->direction(); On 2014/11/27 09:55:04, rune wrote: > ...
6 years ago (2014-11-27 10:03:02 UTC) #8
rune
https://codereview.chromium.org/758073003/diff/20001/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/758073003/diff/20001/Source/core/dom/Document.cpp#newcode1688 Source/core/dom/Document.cpp:1688: rootDirection = bodyStyle->direction(); On 2014/11/27 at 10:03:02, andersr wrote: ...
6 years ago (2014-11-27 10:13:12 UTC) #9
andersr
PTAL. Added new tests.
6 years ago (2014-11-27 10:52:27 UTC) #10
rune
lgtm
6 years ago (2014-11-27 11:42:21 UTC) #11
rune
On 2014/11/27 at 11:42:21, rune wrote: > lgtm Could you modify the description a bit. ...
6 years ago (2014-11-27 11:44:20 UTC) #12
andersr
On 2014/11/27 11:44:20, rune wrote: > On 2014/11/27 at 11:42:21, rune wrote: > > lgtm ...
6 years ago (2014-11-27 12:04:56 UTC) #13
rune
On 2014/11/27 at 12:04:56, andersr wrote: > On 2014/11/27 11:44:20, rune wrote: > > On ...
6 years ago (2014-11-27 12:16:14 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/758073003/40001
6 years ago (2014-11-27 12:29:11 UTC) #16
commit-bot: I haz the power
6 years ago (2014-11-27 13:39:09 UTC) #17
Message was sent while issue was closed. 
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=186115

Powered by Google App Engine
This is Rietveld 408576698