Closed Bug 333817 Opened 18 years ago Closed 18 years ago

Empty select focus rect not painted (and not sized correctly)

Categories

(Core :: Layout: Form Controls, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: roc)

References

()

Details

Attachments

(1 file)

This looks like a regression from bug 192767 (checked by doing builds by date for 
"Wed Mar 15 10:53:00 CST 2006" and "Wed Mar 15 10:57:00 CST 2006" -- things work in the former but not the latter.

To test, just tab around in the testcase.  You should be able to tab to the <select> and get a focus rect painted...  Not sure what'd going on yet, but I can try debugging on Friday, I think.  The select _does_ seem to be in the tab order, just no focus rect painting.

Once this is fixed, I bet I'll be able to reproduce bug 325321 again, so marking dep.
So we're hitting this code in nsSelectsAreaFrame::BuildDisplayListInternal:

208         return aLists.Outlines()->AppendNewToTop(new (aBuilder)
209           nsDisplayGeneric(this, PaintListFocus, "ListFocus"));

But we're never calling PaintListFocus.
Painting --- before optimization (dirty 112,112,560,1428):
SolidColor 0x890ab34(Viewport(-1)) (0,0,13944,9030)
Clip (nil)() (0,0,13944,9030)
    CanvasBackground 0x890abc8(Canvas(html)(-1)) (0,0,13944,9030) uniform
Clip (nil)() (0,0,13944,9030)
    Background 0x891ab9c(ListControl(select)(0)) (112,112,556,1414) opaque uniform
    Border 0x891ab9c(ListControl(select)(0)) (112,112,556,1414)
    Background 0x891ae08(ScrollbarFrame(scrollbar)(-1)) (430,140,210,1358)
    Background 0x891ae78(ButtonBoxFrame(scrollbarbutton)(-1)) (430,140,210,210)
    Border 0x891ae78(ButtonBoxFrame(scrollbarbutton)(-1)) (430,140,210,210)
    Background 0x8920044(ButtonBoxFrame(scrollbarbutton)(-1)) (430,1288,210,210)
    Border 0x8920044(ButtonBoxFrame(scrollbarbutton)(-1)) (430,1288,210,210)
Clip 0x891ac78(Area(select)(0)) (0,0,13944,9030)
    Clip 0x891ac78(Area(select)(0)) (140,140,290,1358)
        ListFocus 0x891ac78(Area(select)(0)) (140,140,0,28)

Painting --- after optimization:
SolidColor 0x890ab34(Viewport(-1)) (0,0,13944,9030)
Clip 0x891ac78(Area(select)(0)) (0,0,13944,9030)
    CanvasBackground 0x890abc8(Canvas(html)(-1)) (0,0,13944,9030) uniform
    Background 0x891ab9c(ListControl(select)(0)) (112,112,556,1414) opaque uniform
    Border 0x891ab9c(ListControl(select)(0)) (112,112,556,1414)
    Background 0x891ae08(ScrollbarFrame(scrollbar)(-1)) (430,140,210,1358)
    Background 0x891ae78(ButtonBoxFrame(scrollbarbutton)(-1)) (430,140,210,210)
    Border 0x891ae78(ButtonBoxFrame(scrollbarbutton)(-1)) (430,140,210,210)
    Background 0x8920044(ButtonBoxFrame(scrollbarbutton)(-1)) (430,1288,210,210)
    Border 0x8920044(ButtonBoxFrame(scrollbarbutton)(-1)) (430,1288,210,210)

So I'd guess the 0 width is the problem here...
So I bet the relevant change (from the checkin comment for bug 192767) is:

  This removes the invariant that the child of a scrollframe (::scrolled-canvas
  canvas or ::scrolled-content block) will be stretched to fill the entire
  scrollable area and stops enforcing it

In this case, the scrolled frame is an nsSelectsAreaFrame; I'll poke at what nsAreaFrame::Reflow does in this case.
In a current trunk build we have:

  ListControl(select)(0)@0x86987e0 [view=0x83748b0] {0,0,556,1414} [state=00046020] [content=0x8712680] [sc=0x8698030]<
    Area(select)(0)@0x86988bc [view=0x869f0d0] {28,28,0,28} [state=00906008] [overflow=0,0,290,1358] sc=0x8698f24(i=0,b=0) pst=:-moz-scrolled-content<

whereas in a build from before bug 192767 we have:

  ListControl(select)(0)@0x86ae9ec [view=0x8793430] {0,0,556,1414} [state=00046020] [content=0x87b3420] [sc=0x86ae2c0]<
    Area(select)(0)@0x86aeac8 [view=0x8796f30] {28,28,290,1358} [state=00906000] sc=0x86af130(i=0,b=0) pst=:-moz-scrolled-content<
So it looks like we reflow the scrolled content with an unconstrained computed width.  It makes sense that it ends up with 0 width, then....

Should we try to work around that somehow?  Or just stop painting focus from the SelectsAreaFrame?  E.g. paint it from the listbox itself?
Summary: Unable to (visibly?) tab to empty select → Empty select focus rect not painted (and not sized correctly)
Flags: blocking1.9a2?
The focus displayitem should remain associated with the nsSelectsAreaFrame --- or even move down to the focused option --- to ensure it gets clipped properly by the scrollframe.

The underlying problem here is that the bounds of the nsSelectsAreaFrame are no longer true bounds for what we'll paint in PaintListFocus. That should be easy for me to fix.
Attached patch fixSplinter Review
Indeed, this fixes it.
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attachment #218755 - Flags: superreview?(bzbarsky)
Attachment #218755 - Flags: review?(bzbarsky)
Comment on attachment 218755 [details] [diff] [review]
fix

Ah, nice.  Makes sense.
Attachment #218755 - Flags: superreview?(bzbarsky)
Attachment #218755 - Flags: superreview+
Attachment #218755 - Flags: review?(bzbarsky)
Attachment #218755 - Flags: review+
checked in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Flags: blocking1.9a2?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: