WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
31122
[Qt] editing/selection/shrink-selection-after-shift-pagedown.html failing
https://bugs.webkit.org/show_bug.cgi?id=31122
Summary
[Qt] editing/selection/shrink-selection-after-shift-pagedown.html failing
Chang Shu
Reported
2009-11-04 08:27:31 PST
QtLauncher failed to pass this newly added test case. It's also reported by build bot:
http://build.webkit.org/results/Qt%20Linux%20Release/r50408%20%283364%29/results.html
Attachments
this patch implements the support for shift-pageup and shift-pagedown.
(1.77 KB, patch)
2009-11-04 11:29 PST
,
Chang Shu
no flags
Details
Formatted Diff
Diff
fix test case
(2.30 KB, patch)
2009-11-05 11:43 PST
,
Chang Shu
no flags
Details
Formatted Diff
Diff
update test description
(2.23 KB, patch)
2009-11-06 11:14 PST
,
Chang Shu
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chang Shu
Comment 1
2009-11-04 11:29:31 PST
Created
attachment 42502
[details]
this patch implements the support for shift-pageup and shift-pagedown. This patch does not make the test case pass yet. But this is the 1st step and better to be in a separate patch.
Tor Arne Vestbø
Comment 2
2009-11-05 06:44:42 PST
Comment on
attachment 42502
[details]
this patch implements the support for shift-pageup and shift-pagedown. LGTM, but please wrap the else before submitting, ie: if (kevent->shiftKey()) frame->editor()->command("MovePageDownAndModifySelection").execute(); else frame->editor()->command("MovePageDown").execute();
Chang Shu
Comment 3
2009-11-05 08:41:31 PST
I think the original test case has a flaw: it issues a shift-pagedown and expects exactly 7 more links are selected. This is true for Safari. However, on other WebKit browsers like QtLauncher or GTK browsers (
bug 31103
), or Firefox, or IE, the amount of selected lines caused by shift-pagedown event is NOT deterministic. This makes the test case fail on other platforms. The intention of the test case is to check if selection would shrink as expected after a shift-arrowup following a "shift-down". I guess this "shift-down" can either be shift-arrowdown or shift-pagedown. While shift-arrowdown generates deterministic behavior, I suggest we change the test to replace shift-pagedown with multiple shift-arrowdown. This should resolve both failures on Qt and GTK. Btw, Qt did not support shift-pageup and shift-pagedown before and this has been addressed in
bug 31166
and fixed already.
Chang Shu
Comment 4
2009-11-05 11:43:56 PST
Created
attachment 42582
[details]
fix test case The original writer of the test case, Enrica Casucci [
enrica@apple.com
], pointed out that shift-PageDown is required for the test case. So my previous proposal of replacing it using multiple shift-arrowdown is not an option. Then I came up with this patch. This patch fixes the test case so that it no longer depends on the exact number of lines selected. On Qt, shift-pagedown has been implemented for
bug 31166
and landed. I tested both Qt and Mac and this patch worked. I also think it should fix
bug 31103
, a GTK regression on this test case.
Enrica Casucci
Comment 5
2009-11-05 11:51:13 PST
The patch looks good to me.
WebKit Commit Bot
Comment 6
2009-11-05 14:47:08 PST
Comment on
attachment 42582
[details]
fix test case Clearing flags on attachment: 42582 Committed
r50579
: <
http://trac.webkit.org/changeset/50579
>
WebKit Commit Bot
Comment 7
2009-11-05 14:47:15 PST
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 8
2009-11-06 09:42:14 PST
This change leaves the test case in an inconsistent state. The text telling how to run the test case says: The selection should only include the lines with "YES". But that's no longer what the test looks like.
Chang Shu
Comment 9
2009-11-06 09:58:11 PST
(In reply to
comment #8
)
> This change leaves the test case in an inconsistent state. The text telling how > to run the test case says: > > The selection should only include the lines with "YES". > > But that's no longer what the test looks like.
I guess I should update this text. However, the reason for inconsistent result is because of the behavior of shift-pagedown.
Chang Shu
Comment 10
2009-11-06 11:14:49 PST
Created
attachment 42662
[details]
update test description
Eric Seidel (no email)
Comment 11
2009-11-06 11:21:54 PST
I think Chang meant to re-open the bug before posting a new patch.
Chang Shu
Comment 12
2009-11-06 11:23:16 PST
(In reply to
comment #11
)
> I think Chang meant to re-open the bug before posting a new patch.
Thanks for doing this for me.
Holger Freyther
Comment 13
2009-11-07 00:49:44 PST
Comment on
attachment 42662
[details]
update test description Sounds sensible.
WebKit Commit Bot
Comment 14
2009-11-08 11:09:48 PST
Comment on
attachment 42662
[details]
update test description Clearing flags on attachment: 42662 Committed
r50626
: <
http://trac.webkit.org/changeset/50626
>
WebKit Commit Bot
Comment 15
2009-11-08 11:09:53 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug