Focus Outline of Select missing left, top, and bottom

RESOLVED FIXED in mozilla1.9beta1

Status

()

Core
Layout
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Ronald Tilby, Assigned: roc)

Tracking

({regression, testcase})

Trunk
mozilla1.9beta1
x86
Windows XP
regression, testcase
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070127 Minefield/3.0a2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070127 Minefield/3.0a2pre

The focus outline (dotted lines) shows only the right side line in simple SELECT elements.

Reproducible: Always

Steps to Reproduce:
1.Load URL (or any page with simple SELECT elements)
2.Tab to the SELECT element
3.Observe that only the right side of the focus outline is shown.
Actual Results:  
Only the right side of the focus outline is visible.

Expected Results:  
The entire focus outline (four dotted lines) should be visible to indicate that the element has focus.

This regressed on 2007-01-17 between Trunk hourly builds 14:08 and 14:32.
(Reporter)

Comment 1

11 years ago
Created attachment 253035 [details]
Testcase

Testcase Passes using: Trunk 2007-01-17 14:08 Hourly

Testcase Fails using: Trunk 2007-01-17 14:32 Hourly
and Current Nightly
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070127 Minefield/3.0a2pre ID:2007012704 [cairo]
Range neatly fits bug 366722.
Component: General → Layout
Keywords: regression
Product: Firefox → Core
QA Contact: general → layout
Version: unspecified → Trunk
(Reporter)

Updated

11 years ago
Keywords: testcase
I'm seeing this bug with the windowsXP theme, not with the classic theme.
Blocks: 366722
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

11 years ago
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
I'm not seeing the bug on trunk as of today (2007-05-23), running with default theme under linux.
(In reply to comment #4)
> I'm not seeing the bug on trunk as of today (2007-05-23), running with default
> theme under linux.
> 

Oops, cancel that comment -- I didn't notice that this bug is WinXP-only.

Martijn's comment #3 still applies.  I tested the 5/23/07 nightly WinXP build of trunk, and the bug occurs specifically when WinXP is using "Windows XP Style" windows and buttons, not "windows classic" style.
Is this a drawing order issue?  Does nsComboboxControlFrame::PaintFocus get called too early (so other things paint on top of it)?  Or is the sizing off somehow?

Comment 7

11 years ago
It seems to me that the refresh/redraw area for outline changes are not calculated correctly when only the 'outline' style changes. 
If also another style changes (forcing reflow?) than the redraw is better in my experience.
Created attachment 282060 [details]
Screen shot comparing FF2 and Minefield-trunk on Windows Vista

This is a screen shot comparing FireFox2 and Minefield on Windows Vista. If you zoom in and look closely, it looks like the combobox's dropdown button is not getting its dark blue/purple border/outline drawn. Is this related? Does this happen on other platforms, or only for Windows Vista?

Comment 9

11 years ago
Another way to reproduce focus redraw issues:
Install and select 'Nautipolis' from AMO.
Restart FF.
Open Tools/Options.
Use Tab key to move focus.
See how the focus ring (which is 2 pixels wide, surrounding the widget) gets drawn only partially.
If you move the move over the widgets, the whole focus ring is correctly drawn (this is probably because other style attributes get changes with :hover, such as background). 

It looks like as if the 'outline' draw is not triggered correctly.
My guess is that what's happening is that certain frame children of the <select> are being given the same style context as the primary frame for the element, which has overflow:-moz-hidden-unscrollable. This clips the content of each such frame to its inner-border-rect. If you remove overflow:-moz-hidden-unscrollable from 'select' in forms.css, I bet the problem goes away. However that is not really the right fix.
(In reply to comment #10)
> If you remove
> overflow:-moz-hidden-unscrollable from 'select' in forms.css, I bet the problem
> goes away.

Indeed, this looks as expected:

data:text/html,<SELECT style="overflow:visible"><OPTION>Foo</OPTION><OPTION>Bar</OPTION></SELECT>

(In reply to comment #11)
> data:text/html,<SELECT
> style="overflow:visible"><OPTION>Foo</OPTION><OPTION>Bar</OPTION></SELECT>

data:text/html,<SELECT style="overflow:visible"><OPTION>Foo<OPTION>Bar</SELECT>
The listbox/combobox-dropdown is an nsListControlFrame, which is a subclass of nsGfxScrollFrame and overrides GetScrollbarStyles, hence ignores its value of 'overflow' for determining its own scrollbar settings. It has one scrollable child frame, an nsSelectsAreaFrame, which actually does not inherit 'overflow' as I had previously hypothesized.

The problem here is that nsFrame::BuildDisplayListForChild, when called on the nsSelectsAreaFrame, is noticing that the parent is overflow:-moz-hidden-unscrollable, and therefore trying to clip the child to the parent's padding-rect. But it computes the padding-rect incorrectly, by taking the parent's border-rect and then subtracting the border taken from the parent's style context (2px on each side). But, on my Mac at least, nsITheme has overridden the parent border to be 1px on each side, so we're actually clipping 1px inside the top, left and bottom of the nsSelectsAreaFrame. (The right escapes because the nsSelectsAreaFrame doesn't extend over to the right edge of the parent, because the scrollbar is in the way.)

So the first thing we need to do is fix ApplyOverflowClipping in BuildDisplayListForChild to use parent->GetPaddingRect, which calls GetUsedBorder to ensure that we compute the padding-rect using overridden border values, if there are any. That's easy.

We should probably also get rid of overflow:-moz-hidden-unscrollable on selects, where it's not really having an effect, and replace it with something more targeted. I'm guessing it's supposed to be preventing the combobox text display and button from overflowing.
Created attachment 282181 [details] [diff] [review]
fix part 1

This fixes ApplyOverflowClipping to use the correct padding-rect, as described in the comment above. Trivial.
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attachment #282181 - Flags: superreview?(dbaron)
Attachment #282181 - Flags: review?(dbaron)
It seems that overflow:-moz-hidden-unscrollable was added to <select> by the reflow branch, so I'm guessing Boris added it. However, I'm not sure why it's needed.

For listboxes, it's superfluous because the listbox is a nsGfxScrollFrame, uses its own scrollbar settings, and clips its kids appropriately.

For comboboxes, it doesn't affect the dropdown at all. nsComboboxControlFrame::Reflow forces the text display frame and the button frame to fit horizontally. They may not fit vertically if the select has a non-auto height --- is that the goal here, to clip the text display frame and the button frame if they overflow vertically?
Comment on attachment 282181 [details] [diff] [review]
fix part 1

r+sr+a1.9=dbaron
Attachment #282181 - Flags: superreview?(dbaron)
Attachment #282181 - Flags: superreview+
Attachment #282181 - Flags: review?(dbaron)
Attachment #282181 - Flags: review+
Attachment #282181 - Flags: approval1.9+
checked in, so marking this fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
nsDisplayBorder::OptimizeVisibility needs a similar change, right?

As for the moz-hidden-unscrollable style, I believe that was the idea yes.  Maybe we should add a comment to that effect....
Created attachment 282199 [details] [diff] [review]
fix nsDisplayBorder::OptimizeVisibility
Attachment #282199 - Flags: superreview?(bzbarsky)
Attachment #282199 - Flags: review?(bzbarsky)
Comment on attachment 282199 [details] [diff] [review]
fix nsDisplayBorder::OptimizeVisibility

r+sr=bzbarsky
Attachment #282199 - Flags: superreview?(bzbarsky)
Attachment #282199 - Flags: superreview+
Attachment #282199 - Flags: review?(bzbarsky)
Attachment #282199 - Flags: review+

Updated

11 years ago
Target Milestone: --- → mozilla1.9 M9
checked that in, thanks.
You need to log in before you can comment on or make changes to this bug.