Bug 49615 - [Qt] Focus ring does not show for image maps
Summary: [Qt] Focus ring does not show for image maps
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 36044
  Show dependency treegraph
 
Reported: 2010-11-16 12:56 PST by Yael
Modified: 2010-11-19 02:31 PST (History)
2 users (show)

See Also:


Attachments
Patch (2.38 KB, patch)
2010-11-16 13:56 PST, Yael
no flags Details | Formatted Diff | Diff
Patch (23.99 KB, patch)
2010-11-17 14:07 PST, Yael
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.