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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1340643004: Stop propagating customscrollbar styles across iframe boundaries. (Closed)

Created:
5 years, 3 months ago by MuVen
Modified:
4 years, 6 months ago
Reviewers:
skobes
CC:
blink-reviews, Rick Byers
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Stop propagating customscrollbar styles across iframe boundaries. Stop propagating customscrollbar styles across iframe boundaries. Currently we are applying the style of ownerLayoutObject style which is throwing uneven behaviour while creatingcustom-scrollbar's for nested IFrames. To Standardize this behaviour create custom-scrollbar if IFrame body/document has ::-webkit-scrollbar selector else create regular scrollbar. BUG=531816 Committed: https://crrev.com/7cbb3a079ea41d5b9d8b2b7cbe35971543b8d501 Cr-Commit-Position: refs/heads/master@{#398951}

Patch Set 1 #

Patch Set 2 : fixing test cases #

Total comments: 3

Patch Set 3 : Fixing failed test cases #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : Added TestCase #

Patch Set 7 : Fixed layout test failure #

Patch Set 8 : #

Patch Set 9 : addressed review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -18 lines) Patch
M third_party/WebKit/LayoutTests/fast/events/scale-and-scroll-iframe-window.html View 1 2 3 4 5 6 8 2 chunks +1 line, -6 lines 0 comments Download
A third_party/WebKit/LayoutTests/scrollbars/custom-scrollbar-not-inherited-by-iframe.html View 1 2 3 4 5 6 7 1 chunk +24 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/scrollbars/custom-scrollbar-not-inherited-by-iframe-expected.txt View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/FrameView.cpp View 1 2 3 4 5 1 chunk +0 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp View 1 2 3 4 5 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 30 (9 generated)
MuVen
PTAL.
5 years, 3 months ago (2015-09-15 13:06:50 UTC) #2
skobes
https://codereview.chromium.org/1340643004/diff/20001/Source/core/frame/FrameView.cpp File Source/core/frame/FrameView.cpp (right): https://codereview.chromium.org/1340643004/diff/20001/Source/core/frame/FrameView.cpp#newcode480 Source/core/frame/FrameView.cpp:480: if (frameLayoutView && frameLayoutView->style()->hasPseudoStyle(SCROLLBAR)) { Your test uses ::-webkit-scrollbar ...
5 years, 3 months ago (2015-09-16 01:49:04 UTC) #3
MuVen
PTAL https://codereview.chromium.org/1340643004/diff/20001/Source/core/frame/FrameView.cpp File Source/core/frame/FrameView.cpp (right): https://codereview.chromium.org/1340643004/diff/20001/Source/core/frame/FrameView.cpp#newcode480 Source/core/frame/FrameView.cpp:480: if (frameLayoutView && frameLayoutView->style()->hasPseudoStyle(SCROLLBAR)) { for these tests,::-webkit-scrollbar ...
5 years, 3 months ago (2015-09-16 11:18:49 UTC) #4
skobes
https://codereview.chromium.org/1340643004/diff/20001/Source/core/frame/FrameView.cpp File Source/core/frame/FrameView.cpp (right): https://codereview.chromium.org/1340643004/diff/20001/Source/core/frame/FrameView.cpp#newcode480 Source/core/frame/FrameView.cpp:480: if (frameLayoutView && frameLayoutView->style()->hasPseudoStyle(SCROLLBAR)) { On 2015/09/16 11:18:49, MuVen ...
5 years, 3 months ago (2015-09-16 15:06:12 UTC) #5
skobes
Also, am I understanding right that the effect of this change is to stop propagating ...
5 years, 3 months ago (2015-09-16 15:10:01 UTC) #6
MuVen
Hi Skobes, PTAL. Thanks,
4 years, 10 months ago (2016-02-18 09:35:50 UTC) #8
skobes
Can you add a test?
4 years, 10 months ago (2016-02-18 17:54:11 UTC) #9
MuVen
Hi Skobes, Sorry for delay. PTAL at the test case added. Thanks, MuVen.
4 years, 6 months ago (2016-06-06 10:26:38 UTC) #10
skobes
For the test filename I would suggest "custom-scrollbar-not-inherited-by-iframe.html". Your test uses an iframe inside another ...
4 years, 6 months ago (2016-06-07 18:40:02 UTC) #11
MuVen
On 2016/06/07 18:40:02, skobes wrote: > For the test filename I would suggest > "custom-scrollbar-not-inherited-by-iframe.html". ...
4 years, 6 months ago (2016-06-08 11:00:20 UTC) #14
skobes
On 2016/06/08 11:00:20, MuVen wrote: > seems like there is a bug when we request ...
4 years, 6 months ago (2016-06-08 19:23:00 UTC) #15
MuVen
On 2016/06/08 19:23:00, skobes wrote: > On 2016/06/08 11:00:20, MuVen wrote: > > seems like ...
4 years, 6 months ago (2016-06-08 19:50:51 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1340643004/220001
4 years, 6 months ago (2016-06-09 16:45:46 UTC) #18
commit-bot: I haz the power
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full ...
4 years, 6 months ago (2016-06-09 16:45:48 UTC) #20
MuVen
Skobes, Im sorry there was a bug in my earlier test case. The one which ...
4 years, 6 months ago (2016-06-09 16:47:13 UTC) #21
skobes
lgtm
4 years, 6 months ago (2016-06-09 17:39:00 UTC) #22
MuVen
On 2016/06/09 17:39:00, skobes wrote: > lgtm Thanks :)
4 years, 6 months ago (2016-06-09 17:53:50 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1340643004/220001
4 years, 6 months ago (2016-06-09 17:54:02 UTC) #25
commit-bot: I haz the power
Committed patchset #9 (id:220001)
4 years, 6 months ago (2016-06-09 18:35:14 UTC) #27
commit-bot: I haz the power
CQ bit was unchecked
4 years, 6 months ago (2016-06-09 18:35:27 UTC) #28
commit-bot: I haz the power
4 years, 6 months ago (2016-06-09 18:38:53 UTC) #30
Message was sent while issue was closed. 
Patchset 9 (id:??) landed as
https://crrev.com/7cbb3a079ea41d5b9d8b2b7cbe35971543b8d501
Cr-Commit-Position: refs/heads/master@{#398951}

Powered by Google App Engine
This is Rietveld 408576698