376981 - [FIX]problem with the find toolbar layout with certain frameset pages (matching text is not visible)

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Wandrille MOUSSEL]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a3) Gecko/20070322 Firefox/2.0 GranParadiso/3.0a3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a3) Gecko/20070322 Firefox/2.0 GranParadiso/3.0a3

On the given URL, the find toolbar comes over the page. So the matching text is not visible because hidden behind the toolbar.

Reproducible: Always

Steps to Reproduce:
1. Go to the URL
2. Click on "To-do List" link on the left frame
3. Hit Apple+F to open the find toolbar
4. Type a search (for example "date")

Actual Results:  
The text is well highlighted but is behind the find toolbar. Even the scrollbar is behind the toolbar.


Expected Results:  
The document should resize automatically so that nothing can be behind the toolbar.

If you reload the page using the "Reload" button (with the find toolbar already open), the page is rendered as expected (nothing is under the toolbar). But if you close the toolbar now, there is a white space down the window (this place should be now part of the page rendering area).
The frameset used on this place has something special I guess to be resizable (maybe a javascript, I didn't look closely)...

Comment 1

[Ria Klaassen (not reading all bugmail)]

WFM with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4pre) Gecko/20070331 BonEcho/2.0.0.4pre

Comment 2

[Samuel Sidler (old account; do not CC)]

I can reproduce this using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070529 Minefield/3.0a5pre. I cannot reproduce using Firefox 2.0.0.4, however. This appears to be a regression.

I don't see any dupes within the the Firefox component, so confirming.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

--> Core::Layout

I don't think this isn't a front-end issue. If you take a look, you'll notice that the Find Bar appears on top of the scrollbar, too, which implies that this is a layout bug, maybe?

It also happens on Windows.

cc'ing dbaron

Comment 4

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

Do you see similar layout problems when resizing the bottom edge of the window?

Comment 5

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

It's a general vertical resizing problem; the regression date matches the reflow branch landing.

Comment 6

[Wandrille MOUSSEL]

I cannot reproduce the bug anymore with this URL using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; fr; rv:1.8.1.4) Gecko/20070515 Firefox/2.0 GranParadiso/3.0a3.
I know this version is older than Mike Beltzner. But if that can help...

Comment 7

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

Further information is not needed.

Comment 8

[Damon Sicore (:damons)]

Targeting/blocking M9 per request by dbaron.

Comment 9

[Mike Schroepfer]

why is this a beta blocker?

Comment 10

[Boris Zbarsky [:bzbarsky]]

So the frame tree here is:

  Viewport
    Canvas
      Area
        HTMLFrameset

When we reflow the Canvas, aReflowState.mFlags.mVResize is true.  But then when we reflow the AreaFrame, we reflow it with an unconstrained height.  So even though it has the NS_FRAME_CONTAINS_RELATIVE_HEIGHT bit set (by the first-ever call to nsHTMLFramesetFrame::Reflow), it doesn't have aReflowState.mFlags.mVResize set.

So nsBlockFrame::ReflowDirtyLines computes selfDirty to false, and the line itself is not dirty, and the frameset doesn't have NS_BLOCK_HAS_CLEAR_CHILDREN (which we check on all block-display frames, by the way!), so we don't reflow it.

We could set  NS_BLOCK_HAS_CLEAR_CHILDREN on the frameset, but that feels like a real hack.

The reason this doesn't bite quirks percentage heights is because in quirks mode we propagate mFlags.mVResize to auto-height containing block kids.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible fix

This seemed cheaper than burdening every block with a check for a frameset first child.  Such a check could be added here as well, if desired.

David, what do you think?

Comment 12

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

It's a beta blocker because the bug is that frameset pages (or a significant subset of them) don't resize vertically, at all.

I'm actually surprised the fix is that simple -- and I also suspect it only fixes some class of pages.  For example, I suspect we'd still display a frameset if somebody puts a frameset inside a body, but this patch wouldn't fix that case.

(What I'd tried a few days ago was having the rootmost frameset mark NS_FRAME_CONTAINS_RELATIVE_HEIGHT all the way up the tree, but that wasn't sufficient because there was no constrained height to cause us to listen to NS_FRAME_CONTAINS_RELATIVE_HEIGHT.)

That said, this patch should fix the bulk of frameset pages, so we should probably just take it.

Comment 13

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

(One thought I had was putting the top level frameset on the viewport frame's fixed list.)

Comment 14

[Boris Zbarsky [:bzbarsky]]

> For example, I suspect we'd still display a frameset if somebody puts a
> frameset inside a body

That's doable, but only using script (or XHTML).  The HTML parser ignores <frameset> once it's got a <body> open.

And yes, this patch wouldn't fix that case.  It also wouldn't fix a frameset inside a table cell, etc.  Of course any such frameset still sizes to the size of the viewport, which is pretty odd behavior in and of itself.  ;)

I would bet that the number of pages creating framesets via the DOM like that is pretty close to 0, by the way.

David, when I check that patch in, should I leave this bug open and assigned to you?  Or resolve it with followups for remaining work (if we want them)?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Hmm.  Comment 13 might in fact work...  That doesn't seem like it would be that hard to do, and it would make a lot more sense than what we do now in some ways.

Comment 16

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

Probably followups.

Thanks for fixing this.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Fixed.  Filed bug 400350.