Closed Bug 326550 Opened 14 years ago Closed 14 years ago

The dots in the focus outline do not match non-cairo builds (draw focus using something other than XOR)

Categories

(Core :: Graphics, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: polidobj, Assigned: masayuki)

References

Details

(Keywords: regression, Whiteboard: cairo)

Attachments

(4 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060208 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060208 Firefox/1.6a1

The focus outline that surrounds a selected link is not the same in Cairo builds as it is in regular trunk builds.

Reproducible: Always

Steps to Reproduce:
1. Select a link e.g. by tabbing to it 
2.
3.

Actual Results:  
The link will have a focus outline around it that has spread out dots.

Expected Results:  
The dots shouldn't be spread out as much.
Whiteboard: cairo
Can you add 2 screenshots to show the difference Brian ?
Attached image Focus Ring comparison
The top is trunk and the bottom is cairo.  Sorry but the difference in the words is the top one is bold.  That's the bold is not bold bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
The difference is not in the spacing of the dots, but in their color. With a non-Cairo build, all the dots are the same color, an inverse of the background color. With a Cairo build, the dots alternate white and black, which on a white background gives the appearance of more widely spaced dots, and on a dark background looks rather ugly. I'll attach a testcase and comparison screenshot.
Notice that the pixels of the focus outline in the non-Cairo build are not black or white but an inverse color to the background.
oh.  I bet this these are The Other place that uses InvertRect.  There is a hack in the tree right now that alternates black and white for InvertRect.

We should draw focus using something other than XOR...
Summary: The dots in the focus outline do not match trunk builds (they are spread out more) → The dots in the focus outline do not match trunk builds(draw focus using something other than XOR)
I've noticed that the tab focus rect looks shit (i.e. cairo hack version), but the focus rects for buttons and radios are fine.

I have to say that the XOR effect is the best I've ever seen for drawing focus rects, and it's standard across all controls and apps on Windows (except tabs & links in Cairo Gecko ;) ).
Summary: The dots in the focus outline do not match trunk builds(draw focus using something other than XOR) → The dots in the focus outline do not match trunk builds (draw focus using something other than XOR)
Summary: The dots in the focus outline do not match trunk builds (draw focus using something other than XOR) → The dots in the focus outline do not match trunk builds(draw focus using something other than XOR)
Flags: blocking1.9a1?
OS: Windows XP → All
Bug 287813 was fixed, by that one, |nsThebesRenderingContext::InvertRect| is using XOR, but it's not works fine on my enviromenent(Win2k + CVS build). The outline is always drawn black (not inverted) on the web contents.
That sounds like the expected behavior based on bug 287813 comment 51.
Yes, it is the expected result from that bug's fix. Now the actual XOR op needs fixing so it really does do XOR, which is what I proposed this bug be used for.
Blake:

Can this fix too?
Attached patch Patch rv1.0Splinter Review
We should use current color when the outline-color is invert.
CSS3 spec said:
http://www.w3.org/TR/css3-ui/#outline-color
> Conformant UAs may ignore the 'invert' value on platforms that do not support color inversion of the pixels on the screen. If the UA does not support the 'invert' value then the initial value of the 'outline-color' property is the 'currentColor' [CSS3COLOR] keyword.

Current cairo doesn't support XOR drawing.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attachment #219606 - Flags: superreview?(roc)
Attachment #219606 - Flags: review?(roc)
vlad, pav, this is the way you guys want to go?
Sure, that sounds like a fine approach to me.  If we figure out how to do invert reasonably at some point we can always change it back; for example, we can always "manually" invert if we're rendering to a surface backed by an in-memory bitmap, e.g. on windows with a DIB.
Attachment #219606 - Flags: superreview?(roc)
Attachment #219606 - Flags: superreview+
Attachment #219606 - Flags: review?(roc)
Attachment #219606 - Flags: review+
checked-in.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: blocking1.9a1?
Target Milestone: --- → mozilla1.9alpha
This is wrong.  If we don't support inversion, we shouldn't parse the 'invert' keyword.
I filed bug 335394 on removing support for 'invert'.
I think this caused SeaMonkey trunk crash bug 335474.
Note also that this change makes outlines a lot less accessible, since it's pretty easy to write HTML that will have invisible focus outlines with this patch.
Note that we happen to be now talking about making the focus outline a lot easier to see, via bug 251198. CC'ing Mark Pilgrim, who's dealing with a11y issues in UI.
Well,that is all wonderfull, but in the meantime, could we have the null checks required added to this patchto prevent the crash reported in bug 335474, or have this patch backed out????
(In reply to comment #21)
> Well,that is all wonderfull, but in the meantime, could we have the null checks
> required added to this patchto prevent the crash reported in bug 335474, or
> have this patch backed out????

The crach will be fixed by the patch of bug 335394. I'm waiting dbaron's review.
Summary: The dots in the focus outline do not match trunk builds(draw focus using something other than XOR) → The dots in the focus outline do not match non-cairo builds (draw focus using something other than XOR)
You need to log in before you can comment on or make changes to this bug.