Bug 26075 - [GTK] Refactor key scrolling code
Summary: [GTK] Refactor key scrolling code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-29 02:36 PDT by Xan Lopez
Modified: 2009-05-29 08:57 PDT (History)
0 users

See Also:

Attachments
scroll.patch (4.06 KB, patch)
2009-05-29 02:38 PDT, Xan Lopez 		no flags Details | Formatted Diff | Diff
scrollv2.patch (3.81 KB, patch)
2009-05-29 04:38 PDT, Xan Lopez 		gustavo: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Lopez 2009-05-29 02:36:46 PDT 
Patch attached with explanation.
Comment 1 Xan Lopez 2009-05-29 02:38:04 PDT 
Created attachment 30770 [details]
scroll.patch
Comment 2 Xan Lopez 2009-05-29 04:34:04 PDT 
This seems to break scrolling in the Acid2 test :/

I thought we were always guaranteed to have a scrollbar, even if it would be a dummy one, but apparently in that case WebCore thinks we don't have one and scrolling fails when using ScrollView::scroll. Guess I'm missing something here?
Comment 3 Xan Lopez 2009-05-29 04:38:34 PDT 
Created attachment 30774 [details]
scrollv2.patch

Be more consistent with our previous implementation, always return true and do not scroll parent views (not sure if that's correct or not). My previous comment still applies though.
Comment 4 Gustavo Noronha (kov) 2009-05-29 08:50:57 PDT 
Comment on attachment 30774 [details]
scrollv2.patch

Looks good to me. Testing in Chromium/Linux and asking people to test on Safari indicates that not scrolling in acid2 is actually the right thing, so this fixes a bug =).
Comment 5 Xan Lopez 2009-05-29 08:57:01 PDT 
Landed as r44264, thanks!