Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
6 years ago by Mathias Hällman Modified:
5 years, 11 months ago Reviewers:
aelias_OOO_until_Jul13, jochen (gone - plz use gerrit) CC:
blink-reviews Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionScroll text field when selecting text
BUG=437284
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188386
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #Patch Set 4 : #Patch Set 5 : #
Created: 5 years, 11 months ago
Messages
Total messages: 28 (10 generated)
mathiash@opera.com changed reviewers: + aelias@chromium.org, jochen@chromium.org
jochen, aelias, could either of you have a look at this? I basically added the UserTriggered flag, as that's what moveCaretSelection (see bug) is doing a few lines down and I can't see how this one can be any less user triggered than that one.
is it possible to add a test for this?
On 2014/11/28 13:08:34, jochen (slow) wrote: > is it possible to add a test for this? Possibly, but I'm not sure what such a test would look like and looking at Source/web/tests/WebFrameTest.cpp there's nothing for moveCaretSelection that I can mimic. Any of you care to give a hint?
I guess the test would need to load in some HTML with a textbox, select text in it, move the selection around and then assert that the textbox's scroll offset has moved.
I haven't been able to prioritize writing a test for this, but here we go! Anyone care to take another look?
lgtm, thanks.
The CQ bit was checked by mathiash@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/766753002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/4...)
The CQ bit was checked by mathiash@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/766753002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/45953)
The CQ bit was checked by mathiash@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/766753002/60001
It seems the test decided to use an odd font on my Arch, so with PS3 I tried tweaking some numbers to match what happens on Ubuntu, and it seems to have worked. Then it failed on windows, however. I'm committing and hoping it's ok that for PS4 I've tried setting font family and size explicitly. Although it seems strange these defaults would be different between the platforms. At least it worked on both my Arch and a coworker's Ubuntu.. So that's something. Anyway if this fails I have to assume it's because of the input fields being native widgets, and I'm not sure how to deal with that in this test. Suggestions would be welcome!
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink ()
The CQ bit was checked by mathiash@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/766753002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_blink_rel on tryserver.blink ()
Ah, I thought integration failed because of the tree closing and reopening a bunch of times.. I see now the test did fail on windows.
The CQ bit was checked by mathiash@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/766753002/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://src.chromium.org/viewvc/blink?view=rev&revision=188386 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
