Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1310323003:
Update Element methods for root layer scrolling. (Closed)
|
Created:
5 years, 4 months ago by skobes Modified:
5 years, 4 months ago Reviewers:
eae CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, sof, eae+blinkwatch, rwlbuis, szager1 Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionUpdate Element methods for root layer scrolling.
Four methods on blink::Element that provide special functionality for the html
element (document.documentElement in JS) are updated for compatibility with root
layer scrolling.
BUG=518885
R=eae@chromium.org
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201166
Patch Set 1 : #
Total comments: 2
Patch Set 2 : Address review comments. #
Messages
Total messages: 31 (14 generated)
Patchset #1 (id:1) has been deleted
skobes@chromium.org changed reviewers: + szager@chromium.org
The CQ bit was checked by skobes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1310323003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1310323003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by skobes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1310323003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1310323003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
On 2015/08/25 03:22:22, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) (I think the failures on mac_chromium_rel_ng are unrelated. The bug I filed got duped into http://crbug.com/524248.)
The CQ bit was checked by skobes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1310323003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1310323003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
skobes@chromium.org changed reviewers: + eae@chromium.org - szager@chromium.org
+eae (szager is OOO today)
On it.
LGTM w/nit https://codereview.chromium.org/1310323003/diff/20001/LayoutTests/fast/scroll... File LayoutTests/fast/scrolling/html-element-client-rect-excludes-scrollbars.html (right): https://codereview.chromium.org/1310323003/diff/20001/LayoutTests/fast/scroll... LayoutTests/fast/scrolling/html-element-client-rect-excludes-scrollbars.html:22: shouldBe(String(htmlElement.clientWidth), String(innerWidth - 20)); If you change this to shouldBe("htmlElement.clientWidth", String(innerWidth - 20)); the generated output will be more readable and look something like: "PASS: htmlElement.clientWidth is 780" instead of "PASS: 780 is 780"
https://codereview.chromium.org/1310323003/diff/20001/LayoutTests/fast/scroll... File LayoutTests/fast/scrolling/html-element-client-rect-excludes-scrollbars.html (right): https://codereview.chromium.org/1310323003/diff/20001/LayoutTests/fast/scroll... LayoutTests/fast/scrolling/html-element-client-rect-excludes-scrollbars.html:22: shouldBe(String(htmlElement.clientWidth), String(innerWidth - 20)); On 2015/08/25 20:42:42, eae wrote: > If you change this to > > shouldBe("htmlElement.clientWidth", String(innerWidth - 20)); > > the generated output will be more readable and look something like: > > "PASS: htmlElement.clientWidth is 780" > instead of > "PASS: 780 is 780" Done.
The CQ bit was checked by skobes@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org Link to the patchset: https://codereview.chromium.org/1310323003/#ps40001 (title: "Address review comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1310323003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1310323003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by skobes@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1310323003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1310323003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Message was sent while issue was closed.
Committed patchset #2 (id:40001) manually as 201166 (presubmit successful). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
