291757 - <br clear=all> has excessive clearance

Status: VERIFIED FIXED

(Core :: Layout: Floats, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

See testcase.
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2738&action=view

the space that is above of the link is excessive.

And if click the link, we can see to reflow.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

If I remove the table element or the first br element, this bug is not reproduced.
And if the text is not wrapped, it is not reproduced too.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

It is reproduced on WinXP and Linux.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to bug 291759 comment #1)
> Could you attach a screen shot of what you think is the bug? I think it is just
> the <br clear=all> that makes the extra space, after all, it is a line break.

Click the testcase's link.
I think that correctly layout is after click.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: testcase2(table is gone)

This patch doesn't reproduce this bug.
I think the previous testcase should be layouted as same.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This bug is reproduced on http://amazon.co.jp/ that is major site.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Bernd:

This bug is reproduced in table cell only.
Please check it.

Comment 7

[Hideo Saito]

Bernd and Masayuki, I think that this problem has same cause as Bug 223737,
since posted attachment 134179 [details] [diff] [review] also has an effect for testcase2.

Comment 8

[Hideo Saito]

not testcase2 but
URL(http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2738&action=view)

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yeah, it cannot be reproduced on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8b2) Gecko/20050620 Firefox/1.0+

-> FIXED

Thank you, Saito-san.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> Reopening. Bug 223737's patch was backed out.

Comment 11

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

I fixed the problem in bug 223737. The testcase here still shows another bug.
What's happening is that as we make the window narrower and more lines of text
appear next to the float, the link moves down because we're not reflowing it (or
the line with <br clear="all">) so we just slide it by the height of the
inserted lines. We should be detecting that the <br clear="all"> has clearance
and we're potentially changing the clearance amount, so we should reflow the
line with that <br>. In fact there is code to do that but it isn't working...

Comment 12

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

Attachment: fix

This fixes the remaining issue. It's a rather stupid bug, sorry --- we need to
record the inline clear status in both the dirty and non-dirty cases, instead
of just the non-dirty case.

amazon.co.jp looks fine to me know, although of course I can't read any of it
:-)

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The patch works fine in amazon.co.jp.
Thanks!

Comment 14

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

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

I don't understand what this code is doing (what a float-related
break-type-after on inline lines actually means, unless it means that the line
following is a block with a float-related break-type-before), but moving the
code seems reasonable, so r+sr=dbaron.

Comment 15

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

It means that the line ended with a <BR> element with "clear" set on it. Thanks!

Comment 16

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

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

Fixes an issue with amazon.co.jp

Comment 17

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

checked in

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

works fine!
thank you!

-> v.