404418 - button gets "button:empty" style when scrolled down, until hovered

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: screenshot: button with too small height

Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b2pre) Gecko/2007111805 Minefield/3.0b2pre - Build ID: 2007111805

Some buttons have a too small height. They get there normal size if the user moves their mouse over it. Going somewhere else with the mouse leaves the button normal.

Comment 1

[Mike Schroepfer]

Thanks for filling a bug - can you submit an HTML testcase as well?

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: html testcase with popupping buttons - was wrong file

Still broken with Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b2pre) Gecko/2007112205 Minefield/3.0b2pre

If it doesn't break with the first load, use ctrl + R.

Comment 3

[Mike Schroepfer]

I'm not seeing the behavior on that testcase - which button is popping for you?

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: testcase: scroll down, press reload

Sorry, I had attached the wrong file.

Steps to reproduce:
1. Open the file.
2. Scroll a few lines down.
3. Press reload.

Comment 5

[Mike Schroepfer]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007120605 Minefield/3.0b2pre ID:2007120605

Yep - see it now with the new testcase.  Thanks!

Comment 6

[Daniel Holbert [:dholbert]]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008013004 Minefield/3.0b3pre

I see this on Linux as well, using the steps & testcase from Comment 4.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase (scroll down, reload, hover over button)

Reduced version of attachment 291197 [details].

The problem seems to be that "button:empty" style gets applied to all buttons on the page whenever you've scrolled down even a tiny bit.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to comment #7)
> The problem seems to be that "button:empty" style gets applied to all buttons
> on the page whenever you've scrolled down even a tiny bit.

... but when you hover over the button, the "button:empty" style is removed.

Try it on the reduced testcase, attachment 300909 [details]:
  1) scroll down, reload (--> strikethrough appears)
  2) hover over button (--> strikethrough disappears)

Switching component to Style, and updating title.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to comment #8)
> ... but when you hover over the button, the "button:empty" style is removed.

Similarly, if you use 'tab' to select the button, the "button:empty" style is removed. (the strikethrough goes away.)

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

My guess is that what you're seeing in the testcase is a combination of bug 98997 and the fact that the presence of state to restore (which can apply to forms, scrollbar positions, etc.) influences when we first resolve style on the button.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yes.  The button ends up matching :empty at this stack:

#0  SelectorMatches (data=@0xbf946208, aSelector=0x9ab6e00, aStateMask=0, 
    aAttribute=0x0, aDependence=0x0)
    at /builds/trunk/mozilla/layout/style/nsCSSRuleProcessor.cpp:1131
...
#6  0x0134c21f in nsStyleSet::ResolveStyleFor (this=0xa01c800, 
    aContent=0xa118898, aParentContext=0xa13a890)
    at /builds/trunk/mozilla/layout/style/nsStyleSet.cpp:652
#7  0x011698d1 in nsCSSFrameConstructor::ResolveStyleContext (this=0x9f6fe50, 
    aParentFrame=<value optimized out>, aContent=0xa118898)
    at /builds/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp:6820
#8  0x01174a3a in nsCSSFrameConstructor::ConstructFrame (this=0x9f6fe50, 
    aState=@0xbf946384, aContent=0xa118898, aParentFrame=0xa13aac0, 
    aFrameItems=@0xbf94648c)
    at /builds/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp:7559
#9  0x011810a4 in nsCSSFrameConstructor::ContentAppended (this=0x9f6fe50, 
    aContainer=0x9f24f80, aNewIndexInContainer=0)
    at /builds/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp:8732
...
#16 0x0140ed5d in nsDocument::FlushPendingNotifications (this=0xa1178c0, 
    aType=Flush_ContentAndNotify)
    at /builds/trunk/mozilla/content/base/src/nsDocument.cpp:4813
#17 0x013e0272 in nsContentList::BringSelfUpToDate (this=0x9f46738, aDoFlush=1)
    at /builds/trunk/mozilla/content/base/src/nsContentList.cpp:846
#18 0x013de53c in nsContentList::IndexOf (this=0x9f46738, aContent=0xa118898, 
    aDoFlush=1) at /builds/trunk/mozilla/content/base/src/nsContentList.cpp:426
#19 0x013f003b in nsContentUtils::GenerateStateKey (aContent=0xa118898, 
    aDocument=0xa1178c0, aID=nsIStatefulFrame::eNoID, aKey=@0xbf946930)
    at /builds/trunk/mozilla/content/base/src/nsContentUtils.cpp:1864
#20 0x014cdfb8 in nsGenericHTMLElement::GetLayoutHistoryAndKey (
    aContent=0xa118898, aRead=1, aHistory=0xbf946988, aKey=@0xbf946930)
    at /builds/trunk/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1605
#21 0x014ce099 in nsGenericHTMLElement::RestoreFormControlState (
    aContent=0xa118898, aControl=0xa1188b8)
    at /builds/trunk/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1631
#22 0x014e1618 in nsHTMLButtonElement::DoneCreatingElement (this=0xa118898)
    at /builds/trunk/mozilla/content/html/content/src/nsHTMLButtonElement.cpp:570

which is why being scrolled down matters.  Otherwise, it's just bug 98997.

Comment 12

[Boris Zbarsky [:bzbarsky]]

So this regressed as a result of fixing bug 398243.  That change makes us flush the content sink when restoring form control state for a form control that is not in a form.

This code has sort of been a mess all along.  No matter how you slice it, there's a perf hit here.  :(

That said, I have to wonder... why do we save/restore the disabled state for buttons, exactly?  Getting rid of that would fix this particular testcase, as would fixing bug 98997 or somehow fixing the state key thing to deal with the content sink notifications not being flushed or something.

I do wish that there were a way to flush out content sink notifications in general but _not_ trigger frame construction.  :(

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix by checkin of bug 401291 to trunk, 2008-02-18 22:17 -0800 (which fixed bug 98997).

Boris, did you want to file bugs on any of the other issues you mentioned?

Comment 14

[Boris Zbarsky [:bzbarsky]]

I think we already more or less have bugs on them...

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b4pre) Gecko/2008022104 Minefield/3.0b4pre - Build ID: 2008022104

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Sorry, I still find the problem, i.e. with the buttons in the Brief ( http://brief.mozdev.org ) extension. The testcase is working fine fore me.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Could you file a separate bug on that, with enough information to reproduce the problem?