Bug 54639 - Navigating downwards / upwards does not focus on the links spread across more than one line.
Summary: Navigating downwards / upwards does not focus on the links spread across more...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: S60 Hardware S60 3rd edition
: P1 Blocker
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2011-02-17 05:06 PST by deepak
Modified: 2011-02-25 11:58 PST (History)
4 users (show)

See Also:

Attachments
widget (2.35 KB, application/x-zip-compressed)
2011-02-17 08:35 PST, Murali Alluri 		no flags Details
Test page attached. (2.39 KB, text/html)
2011-02-17 09:26 PST, deepak 		no flags Details
Patch. (10.18 KB, patch)
2011-02-18 05:25 PST, Yael 		tonikitoo: review+
yael: commit-queue+
Details | Formatted Diff | Diff
Backport (10.60 KB, patch)
2011-02-22 14:42 PST, Yael 		no flags Details | Formatted Diff | Diff
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 deepak 2011-02-17 05:06:42 PST 
Attachment : includes a webpage, that has link that is spread across two lines.

Steps:
1. Open the page, and navigate use key pad in the mobile hardware to navigate the links.

Expected Result:
The link that is spread across two lines also should be selected and focused.

Actual REsult:
The link that is spread across two lines is skipped and the next link is selected and focused.
Comment 1 deepak 2011-02-17 05:08:42 PST 
Additional Info: Tested this with QTTestBrowser, and it is reproducible on the same.
Comment 2 deepak 2011-02-17 05:42:08 PST 
Additional Info:
Since QTTestBrowser does not support key pad navigation, enabled the setting through code in webpage.

this->settings()->setAttribute(QWebSettings::SpatialNavigationEnabled, true);

After which, was able to test this test case.
Comment 3 Yael 2011-02-17 07:30:59 PST 
(In reply to comment #0)
> Attachment : includes a webpage, that has link that is spread across two lines.
> 
> Steps:
> 1. Open the page, and navigate use key pad in the mobile hardware to navigate the links.
> 
> Expected Result:
> The link that is spread across two lines also should be selected and focused.
> 
> Actual REsult:
> The link that is spread across two lines is skipped and the next link is selected and focused.

Please include the attachment.
Comment 4 Murali Alluri 2011-02-17 08:35:45 PST 
Created attachment 82814 [details]
widget
Comment 5 deepak 2011-02-17 09:26:55 PST 
Created attachment 82820 [details]
Test page attached.
Comment 6 Yael 2011-02-17 18:06:21 PST 
Removing [Qt][S60] as the fix is in common code. 
Patch is coming soon.
Comment 7 Yael 2011-02-18 05:25:41 PST 
Created attachment 82946 [details]
Patch.

When 2 anchor elements span more than one line each, and one ends on the same line that the 
second starts on, the rects reported by their renderers are overlapping. When handling 2 overlapping nodes, check for this case, and don't assume that one of the nodes is on a higher layer.
Comment 8 Yael 2011-02-18 06:49:52 PST 
Comment on attachment 82946 [details]
Patch.

Thanks for the review :)
Comment 9 Yael 2011-02-18 13:07:46 PST 
Committed r79021: <http://trac.webkit.org/changeset/79021>
Comment 10 Suresh Voruganti 2011-02-21 07:21:45 PST 
Please cherry pick the fix for Qtwebkit 2.1
Comment 11 Ademar Reis 2011-02-21 12:00:06 PST 
(In reply to comment #10)
> Please cherry pick the fix for Qtwebkit 2.1

The code requires significant changes to be cherry-picked. We need a backport targeted at qtwebkit-2.1.
Comment 12 Yael 2011-02-22 14:42:29 PST 
Created attachment 83393 [details]
Backport

This is a backport of the original patch to qtwebkit-2.1.x
Comment 13 Ademar Reis 2011-02-25 11:58:05 PST 
Revision r79021 cherry-picked into qtwebkit-2.1 with commit 3721aa1 <http://gitorious.org/webkit/qtwebkit/commit/3721aa1>