Bug 20099

Summary:

[GTK] SHIFT+PAGE_UP/DOWN doesn't extend selection

Product:

WebKit

Reporter:

Adrien Nader <camaradetux>

Component:

WebKitGTK

Assignee:

Adrien Nader <camaradetux>

Status:

RESOLVED FIXED

Severity:

Minor

Keywords:

Gtk

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Trivial patch changing call to frame->selection()->modify instead of frame->editor()->command	none
new version with the proper granularity (simpler btw)	sam: review-
Same as before plus the changelog entry	eric: review+

Adrien Nader

Reported 2008-07-18 13:49:50 PDT

Using SHIFT and UP/DOWN or HOME/END correctly extends selection but not with PAGE_UP/PAGE_DOWN. There's no bug, just no implementation. I'm attaching a trivial patch which is basically an adaptation of the code for up/down which is written just before.

Attachments
Trivial patch changing call to frame->selection()->modify instead of frame->editor()->command (1.09 KB, patch) 2008-07-18 13:55 PDT, Adrien Nader	no flags	Details Formatted Diff Diff
new version with the proper granularity (simpler btw) (988 bytes, patch) 2008-07-22 10:48 PDT, Adrien Nader	sam: review-	Details Formatted Diff Diff
Same as before plus the changelog entry (1.41 KB, patch) 2008-08-22 10:17 PDT, Adrien Nader	eric: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Adrien Nader

Comment 1 2008-07-18 13:55:52 PDT

Created attachment 22374 [details] Trivial patch changing call to frame->selection()->modify instead of frame->editor()->command A fairly simple patch which is basically a copy of what was written for the handling of up/down. I'm not sure the proper granularity is ParagraphGranularity though.

Pierre-Luc Beaudoin

Comment 2 2008-07-22 07:51:05 PDT

Based on GEdit's behavior, ParagraphGranularity isn't ok. But considering that Firefox doesn't implement this, it might be ok.

Adrien Nader

Comment 3 2008-07-22 10:48:59 PDT

Created attachment 22431 [details] new version with the proper granularity (simpler btw) The granulariy is now correct. The proper thing was already available but I was not aware of its existence. The patch is also simpler. Basically this patch changes "MovePageUp" to kevent->shiftKey() ? "MovePageUpAndModifySelection" : "MovePageUp"

Sam Weinig

Comment 4 2008-07-26 14:55:18 PDT

Comment on attachment 22431 [details] new version with the proper granularity (simpler btw) This looks ok, but it needs a changelog, and depending on the state of DRT for the GTK port a regression test as well.

Christian Dywan

Comment 5 2008-07-26 15:06:14 PDT

The new lines are indentend with tabs, but you should only use spaces in WebCore. When you address Sam's comments, please fix that as well. Otherwise the patch works as expected from a little testing.

Adrien Nader

Comment 6 2008-08-22 10:17:52 PDT

Created attachment 22942 [details] Same as before plus the changelog entry Finally updated the patch with a changelog and spaces (damn default configuration !).

Eric Seidel (no email)

Comment 7 2008-08-27 16:14:15 PDT

Comment on attachment 22942 [details] Same as before plus the changelog entry I'm assuming that it's impossible to test this with DumpRenderTreeGtk. Looks sane enough.

Holger Freyther

Comment 8 2008-08-27 18:42:27 PDT

Looked sane and landed in r35958.

Note You need to log in before you can comment on or make changes to this bug.