Bug 50325 - [Qt] Fix focus ring outline color support
Summary: [Qt] Fix focus ring outline color support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P4 Minor
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-12-01 08:55 PST by Csaba Osztrogonác
Modified: 2010-12-06 06:51 PST (History)
4 users (show)

See Also:


Attachments
Patch. (150.67 KB, patch)
2010-12-02 17:49 PST, Yael
no flags Details | Formatted Diff | Diff
Patch. (151.24 KB, patch)
2010-12-03 13:36 PST, Yael
no flags Details | Formatted Diff | Diff
Patch. (127.37 KB, patch)
2010-12-06 05:50 PST, Yael
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2010-12-01 08:55:24 PST
Three tests introduced in http://trac.webkit.org/changeset/72962 ,
which use color focus ring outline, but Qt doesn't support it.

fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map.html
fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map.html
fast/images/imagemap-focus-ring-outline-color.html
Comment 1 Csaba Osztrogonác 2010-12-01 09:05:53 PST
Failing tests were added to the skipped list: http://trac.webkit.org/changeset/73027
Comment 2 Csaba Osztrogonác 2010-12-01 09:41:48 PST
Yael, you implemented focus ring support previously,
could you help me with this bug?

It seems there aren't color focus ring, because of Qt::DotLine 
styled pen without width.

I tried the following modification, and it works for me:
nPen.setWidth(3);
nPen.setStyle(Qt::SolidLine);

Is there any reason why we use very thin dotline?
Comment 3 Yael 2010-12-01 10:48:06 PST
(In reply to comment #2)
> Yael, you implemented focus ring support previously,
> could you help me with this bug?
> 
> It seems there aren't color focus ring, because of Qt::DotLine 
> styled pen without width.
> 
> I tried the following modification, and it works for me:
> nPen.setWidth(3);
> nPen.setStyle(Qt::SolidLine);
> 
> Is there any reason why we use very thin dotline?

My initial implementation was following what we do for links. I was going to follow up and change the way we do both, but then I saw that Reni is already working on it in https://bugs.webkit.org/show_bug.cgi?id=49953 .
If she is not working on it, I'll be happy to do that
Comment 4 Yael 2010-12-02 17:49:17 PST
Created attachment 75446 [details]
Patch.

Take into account the width of the pen when drawing the focus ring for image map.
Change the outline style from dotted to solid, since the dotted line does not make sense anymore.
I will file separate bugs to do the same for links' focus ring and for supporting outline-style.
Comment 5 Yael 2010-12-02 19:00:44 PST
When the outline is thick, on mac, they use a gradient and not a solid line. I'd like to see if Qt has the ability to do something similar.
Comment 6 Yael 2010-12-03 13:36:17 PST
Created attachment 75536 [details]
Patch.

Take into account the width of the pen when drawing the focus ring for image map.
Change the outline style from dotted to solid, since the dotted line does not make sense anymore.
Set the alpha to 50% on the focus ring, like other ports do.

I did not change the default color of the focus ring, because I could not find a QStyle API for this.
Comment 7 Yael 2010-12-05 16:58:14 PST
Some of the png images seem to have a problem. Do I need to set svn:eol-style=native on png files ?
Comment 8 Yael 2010-12-06 05:50:26 PST
Created attachment 75684 [details]
Patch.

Update patch after fixing svn:mime-type for the images.
Comment 9 Andreas Kling 2010-12-06 06:14:49 PST
Comment on attachment 75684 [details]
Patch.

r=me
Comment 10 Yael 2010-12-06 06:51:35 PST
Committed r73361: <http://trac.webkit.org/changeset/73361>