Summary: | The default backspace event handler should mark the event as handled if navigation succeeds | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ananta Iyengar <ananta> | ||||||||||
Component: | WebKit Misc. | Assignee: | Ananta Iyengar <ananta> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ap, commit-queue | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Ananta Iyengar
2011-04-12 14:53:50 PDT
Created attachment 89282 [details]
Initial patch
Comment on attachment 89282 [details] Initial patch View in context: https://bugs.webkit.org/attachment.cgi?id=89282&action=review > Source/WebCore/ChangeLog:3 > + https://bugs.webkit.org/show_bug.cgi?id=58379 Please add bug title before the link. > Source/WebCore/ChangeLog:9 > + No new tests. (OOPS!) The patch cannot be landed with OOPS, so you need to explain why there are no new tests instead of leaving the line as is. Created attachment 89285 [details]
Patch with review comments addressed
Comment on attachment 89285 [details] Patch with review comments addressed View in context: https://bugs.webkit.org/attachment.cgi?id=89285&action=review > Source/WebCore/ChangeLog:3 > + https://bugs.webkit.org/show_bug.cgi?id=58379 Still no bug title. > Source/WebCore/ChangeLog:9 > + No new tests added as there is no change in functionality. This is not accurate. The patch fixes a bug, so of course there is change in functionality. Created attachment 89294 [details]
Explanation added as to why there are no tests. Added the title before the bug link as requested.
Comment on attachment 89294 [details]
Explanation added as to why there are no tests. Added the title before the bug link as requested.
Ugh, sorry that I didn't notice it before. You have removed the Not Reviewed line that was added by prepare-ChangeLog, so commit queue won't be able to put the reviewer name in. Please add it back (I suggest running prepare-ChangeLog again to see how exactly it should look).
Created attachment 89304 [details]
Created a new patch after running the prepare-changelog script. Sorry about removing the Reviewed by line
Comment on attachment 89304 [details]
Created a new patch after running the prepare-changelog script. Sorry about removing the Reviewed by line
In normal case, it feels weird to let the event fly further if we happen to be at the first or last page, but block it otherwise. But this is what we do elsewhere in this file, so it's perfectly fine for this patch.
Comment on attachment 89304 [details] Created a new patch after running the prepare-changelog script. Sorry about removing the Reviewed by line Clearing flags on attachment: 89304 Committed r83688: <http://trac.webkit.org/changeset/83688> All reviewed patches have been landed. Closing bug. |