Bug 33714 - [Qt] QWebSettings attribute for toggle Spatial Navigation on/off
Summary: [Qt] QWebSettings attribute for toggle Spatial Navigation on/off
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Antonio Gomes
URL:
Keywords: Qt
Depends on: 18662
Blocks: 31552 33715
  Show dependency treegraph
 
Reported: 2010-01-15 05:29 PST by Antonio Gomes
Modified: 2010-03-05 08:35 PST (History)
5 users (show)

See Also:


Attachments
patch 0.1 (2.62 KB, patch)
2010-01-15 05:33 PST, Antonio Gomes
tonikitoo: commit-queue-
Details | Formatted Diff | Diff
patch 0.2 (3.83 KB, patch)
2010-01-15 12:28 PST, Antonio Gomes
hausmann: review-
tonikitoo: commit-queue-
Details | Formatted Diff | Diff
(committed in r55579) patch 0.3 (4.10 KB, patch)
2010-01-18 19:35 PST, Antonio Gomes
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2010-01-15 05:29:05 PST
This bug is about adding to QWebSettings support to toggle "Spatial Navigation" (SNav) on and off.

SNav itself is being implemented in bug 18662.
Comment 1 Antonio Gomes 2010-01-15 05:33:08 PST
Created attachment 46674 [details]
patch 0.1
Comment 2 WebKit Review Bot 2010-01-15 05:35:30 PST
Attachment 46674 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/190174
Comment 3 Antonio Gomes 2010-01-15 05:38:26 PST
(In reply to comment #2)
> Attachment 46674 [details] did not build on qt:
> Build output: http://webkit-commit-queue.appspot.com/results/190174

I will ignore the build error for now, because this patch depends on patch in bug 18662.
Comment 4 Antonio Gomes 2010-01-15 12:28:40 PST
Created attachment 46698 [details]
patch 0.2

better docs
Comment 5 WebKit Review Bot 2010-01-15 12:31:49 PST
Attachment 46698 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/190451
Comment 6 Simon Hausmann 2010-01-16 00:50:11 PST
Comment on attachment 46698 [details]
patch 0.2

Antonio, I think your patch looks fine, but I would prefer a slightly more elaborate explanation in the docs on what the _effect_ of enabling this feature is, i.e. that it affects the keyboard navigation.
Comment 7 Antonio Gomes 2010-01-16 11:36:06 PST
thx for reviewing, simon. I will improve the patch.

(In reply to comment #6)
> (From update of attachment 46698 [details])
> Antonio, I think your patch looks fine, but I would prefer a slightly more
> elaborate explanation in the docs on what the _effect_ of enabling this feature
> is, i.e. that it affects the keyboard navigation.

@simon: something else that came to mind: as is, the patch will enable/disable SNav per-page, i.e. if two windows are opened at the same time, one can be with SNav ON while it is OFF in other. Is that what we want or we want it to be set a globally  ?
Comment 8 Antonio Gomes 2010-01-18 19:35:10 PST
Created attachment 46879 [details]
(committed in r55579) patch 0.3

Added a more detailed explanation about effects on enabling/disabling SNav.
Comment 9 Simon Hausmann 2010-01-19 01:24:00 PST
Comment on attachment 46879 [details]
(committed in r55579) patch 0.3

Looks good to me. Thanks Antonio for the improved docs :)
Comment 10 Eric Seidel (no email) 2010-02-02 14:18:07 PST
Ping?  This was r+'d two weeks ago.
Comment 11 Antonio Gomes 2010-02-02 17:20:59 PST
(In reply to comment #10)
> Ping?  This was r+'d two weeks ago.

eric, this patch can land before its blocker (bug 18662), which pends review.
Comment 12 Antonio Gomes 2010-02-02 17:23:55 PST
(In reply to comment #11)
> (In reply to comment #10)
> > Ping?  This was r+'d two weeks ago.
> 
> eric, this patch can land before its blocker (bug 18662), which pends review.

s/can/can not/gc

sorry for spamming it.
Comment 13 Antonio Gomes 2010-02-09 06:46:13 PST
Comment on attachment 46879 [details]
(committed in r55579) patch 0.3

Clearing r+/cq- since this patch depends on another bug to be landed first.

so it does not appear on pending-commit page.
Comment 14 Antonio Gomes 2010-03-05 08:32:15 PST
landed in r55579

thx for reviewing you simon.
Comment 15 Antonio Gomes 2010-03-05 08:33:43 PST
Comment on attachment 46879 [details]
(committed in r55579) patch 0.3

clearing flags