Bug 49615

Summary: [Qt] Focus ring does not show for image maps
Product: WebKit Reporter: Yael <yael>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 36044    
Attachments:
Description Flags
Patch
none
Patch none

Description Yael 2010-11-16 12:56:20 PST
http://trac.webkit.org/changeset/53857 added support for focus rings on imagemaps, but each port needs to implement
GraphicsContext::drawFocusRing(const Vector<Path>& paths, int width, int offset, const Color& color) in order to see a focus ring.
Comment 1 Yael 2010-11-16 13:56:48 PST
Created attachment 74039 [details]
Patch
Comment 2 Laszlo Gombos 2010-11-16 19:03:41 PST
Comment on attachment 74039 [details]
Patch

r=me.

It seems that a fair amount of code could potentially be shared between the two GraphicsContext::drawFocusRing functions (and perhaps get rid if the "if 0" guard in the already landed code).

Yael, after landing this, can you open a new bug for sharing the code ?
Comment 3 Yael 2010-11-17 05:27:06 PST
Comment on attachment 74039 [details]
Patch

Thanks for the review, Laszlo,
I'll do the cleanup next.
Comment 4 Antonio Gomes 2010-11-17 06:00:31 PST
why no tests? :(
Comment 5 Yael 2010-11-17 06:01:37 PST
(In reply to comment #2)
> (From update of attachment 74039 [details])
> r=me.
> 
> It seems that a fair amount of code could potentially be shared between the two GraphicsContext::drawFocusRing functions (and perhaps get rid if the "if 0" guard in the already landed code).
> 
> Yael, after landing this, can you open a new bug for sharing the code ?

Unfortunately, we cannot enable the "#if 0" for drawing the focus ring around links. It gives very bad results. e.g. when a link spans multiple lines, the path between those lines is garbled.
Comment 6 Yael 2010-11-17 06:02:39 PST
(In reply to comment #4)
> why no tests? :(

I am about to enable the test LayoutTests/fast/images/imagemap-focus-ring.html for this.
Comment 7 Antonio Gomes 2010-11-17 06:04:00 PST
Comment on attachment 74039 [details]
Patch

It *must* land together or should be in the changelog.
Comment 8 Yael 2010-11-17 14:07:39 PST
Created attachment 74154 [details]
Patch

Same patch as before, with updated test result.
Comment 9 Eric Seidel (no email) 2010-11-18 03:20:15 PST
Comment on attachment 74039 [details]
Patch

Cleared Laszlo Gombos's review+ from obsolete attachment 74039 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 10 WebKit Commit Bot 2010-11-19 02:06:23 PST
The commit-queue encountered the following flaky tests while processing attachment 74154 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html
http/tests/loading/deleted-host-in-resource-load-delegate-callback.html

Please file bugs against the tests.  These tests were authored by beidson@apple.com and dumi@chromium.org.  The commit-queue is continuing to process your patch.
Comment 11 WebKit Commit Bot 2010-11-19 02:31:44 PST
Comment on attachment 74154 [details]
Patch

Clearing flags on attachment: 74154

Committed r72371: <http://trac.webkit.org/changeset/72371>
Comment 12 WebKit Commit Bot 2010-11-19 02:31:50 PST
All reviewed patches have been landed.  Closing bug.