301026 - {inc}Clicking on link in floated element causes the element to decrease its width

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Thomas Bertels]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8b4) Gecko/20050715 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8b4) Gecko/20050715 Firefox/1.0+

Noticed too with Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8b4)
Gecko/20050715 Firefox/1.0+, didn't test it with other versions.

Reproducible: Always

Steps to Reproduce:
1.Scroll down in linked page to the images
2.Click (right or left) on one of the images (which open a pop up window)
Actual Results:  
The text is wrapped at each click until each line isn't larger than the image.

Expected Results:  
No modification of the layout.

You can notice on this other page
(http://extensions.geckozone.org/AllInOneSearchButton) that it's possible to
reset the layout (which changed by clicking on the image of the right) by
clicking on one of the other images (the left or center one, whos text isn't
larger than the image).

Comment 1

[José Jeria]

Attachment: Minimized testcase

Comment 2

[José Jeria]

Note that it's the floated div width that decreases for each click

Comment 3

[Andrew Schultz]

actually, this regressed from (or was at least exposed by) bug 270804

Comment 4

[José Jeria]

Attachment: Clearer testcase

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Pretty simple ... we need to include the right margin in the block's XMost when
we're recovering state. We do this inside ReflowBlockFrame already for the
blocks we reflow. That actually adds it to the *bounds* which is pretty bogus
...

This will go away with reflow-refactor

Comment 6

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

Why is it bogus to add it to the line's bounds?  (It makes sense to me.)  And if
it's added to the line's bounds, why do you need to add it again here?  and why
wouldn't you need to do the same in PostPlaceLine for consistency?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #6)
> Why is it bogus to add it to the line's bounds?  (It makes sense to me.)

Because the line bounds are the line's border-box in all cases (assuming inlines
are wrapped in an anonymous line box with no border/padding) except that
sometimes the right edge includes the right margin.

> And if it's added to the line's bounds, why do you need to add it again here?

Every line that gets the right margin added during SHRINKWRAP reflow gets
reflowed again during ComputeFinalSize, without SHRINKWRAP, so the bounds are
recomputed without the right margin. In other words, the right margin addition
is only temporary.

> and why wouldn't you need to do the same in PostPlaceLine for consistency?

PostPlaceLine is only called from PlaceLine and ReflowBlockFrame. In the latter
case, the right margin has already been added to the bounds. In the former case,
PlaceLine is only for lines of inlines, which can't have a right margin.

Comment 8

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

(In reply to comment #7)
> > Why is it bogus to add it to the line's bounds?  (It makes sense to me.)
> 
> Because the line bounds are the line's border-box in all cases (assuming inlines
> are wrapped in an anonymous line box with no border/padding) except that
> sometimes the right edge includes the right margin.

I still don't follow.  For inline lines there's no distinction, and for block
lines it seems like we may as well use the margin box for horizontal aspects --
presumably we do already for margin-left.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #8)
> I still don't follow.  For inline lines there's no distinction, and for block
> lines it seems like we may as well use the margin box for horizontal aspects --
> presumably we do already for margin-left.

We don't. The bounds.x is the block's border-edge.

I would also argue that having the bounds be the margin edges horizontally and
the border edges vertically is already borderline-bogus :-) I actually did start
work on a patch along those lines --- but less bogusly, removing mBounds in
favour of mBorderBoxTop, mBorderBoxHeight, mMarginBoxLeft, mMarginBoxWidth, but
it got too invasive for this stage of the game.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 189739 [details] [diff] [review]
fix

fix for incremental reflow regression

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using the testcase at
https://bugzilla.mozilla.org/attachment.cgi?id=189595 with build 2005-08-10-05
on Windows XP SeaMonkey trunk.