71979 - <br><hr> inside link displays additional hr upon each mouseover

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Leston Buell]

When you have the following construct:
   <a href="">text<br><hr></a>
an additional horizontal rule is displayed each time the link is moused over.
(See test case for exact code.) 

You can observe this in a real web page at <http://www.advocate.com> in the
right sidebar "news" column.

I have created a test case, which i will attach momentarily.

This is very bizarre.

Comment 1

[Leston Buell]

Attachment: test case

Comment 2

[Eric S. Smith]

Seeing this with 2001031204 on NT.

Comment 3

[Asko Tontti]

Seeing also, PC Linux 2001031408.

Comment 4

[Keyser Sose]

Confirmed
platform: PC
OS: Windows 98
Mozilla Build: 2001031204

Marking NEW.

Comment 5

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

Ian - Are we using :before or :after for BR or HR now, or is this just a related
bug to the problem I'm remembering?

Comment 6

[karnaze (gone)]

Reassigning to waterson and marking m0.9.1.

Comment 7

[Hixie (not reading bugmail)]

We use :before and :after for <hr> elements in quirks mode. My patch for the
HR/BR bug would remove this (and use :before for <br> elements).

I would say this is a major bug, not minor at all! :-O

This is almost certainly a dup of bug 69398.

Comment 8

[Oliver Klee]

*** Bug 84484 has been marked as a duplicate of this bug. ***

Comment 9

[Andreas M. "Clarence" Schneider]

*** Bug 84904 has been marked as a duplicate of this bug. ***

Comment 10

[Andreas M. "Clarence" Schneider]

Comments from dupe:

This is caused by "<br> <hr>" inside <a>. Whitespace between the tags
is necessary to reproduce it. It is quirks mode only.

Another testcase: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37795

Comment 11

[Emlyn Shannon]

If you liked this bug, you might also enjoy bug 85012

Comment 12

[Fabian Guisset]

*** Bug 85012 has been marked as a duplicate of this bug. ***

Comment 13

[Chris Waterson]

Sigh, yet one more reason to fix 38370. This doesn't appear in standard mode.
I'm not going to try to wallpaper over this one, and will just mark it dependent
on that bug.

Comment 14

[basic]

with build 2001070108 win32 trunk I no longer see the bug in the testcase.

Comment 15

[basic]

Okay, I don't see the bug anymore because mozilla nolonger apply 
:before{content: "\A";} or :after{content: "\A";} to hr and br. When I do, the
bug appears. I'll post a testcase.

Comment 16

[basic]

Attachment: br testcase

Comment 17

[basic]

Attachment: hr testcase

Comment 18

[basic]

Attachment: br hr testcase

Comment 19

[Oliver Klee]

*** Bug 93372 has been marked as a duplicate of this bug. ***

Comment 20

[Mats Palmgren (inactive)]

*** Bug 93441 has been marked as a duplicate of this bug. ***

Comment 21

[Mats Palmgren (inactive)]

*** Bug 98140 has been marked as a duplicate of this bug. ***

Comment 22

[kinmoz]

I have a fix in bug 96455 that fixes the problem for HR. It should work for the 
BR case also, but I'm seeing what I think is a bug where asking for the 
PrimaryFrame for a BR with :before and :after content, returns the :before frame 
instead of the nsBRFrame. This results in just the :before frame being removed 
from the frame tree, instead of all three frames (:before, nsBRFrame, and :after 
frame) being removed ... so in the testcases above, you end up with multiple 
:after frames for the br, as you hover in and out of the div. 

Comment 23

[Taral]

This problem exists on my system (galeon 0.12.4 gecko 20011019). It pops up in
malformed directory indexes sometimes -- <hr> in link because someone misplaced
an </a> tag.

Comment 24

[kinmoz]

I have a fix for the problem where GetPrimaryFrame() is returning the :before
frame, instead of the nsBRFrame for br nodes. That fix in combination with the
fix for bug 96455, seems to fix all the test cases attatched to this bug.

I'll take this off of waterson's plate, and try to get them both in for 0.9.7.

Comment 25

[kinmoz]

Attachment: Patch Rev 1 (Return the correct PrimaryFrame for a BR with :before)

The fix for bug 96455 has been checked into the trunk.

Here's the patch that fixes the remaining problem with GetPrimaryFrameFor() not
returning the correct frame when a BR has :before pseudo content. Once this
patch goes in, all the test cases in this bug should work, and we can close out
this bug.

Comment 26

[Chris Waterson]

sr=waterson

Comment 27

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

Hmmm.  You never use |nextSiblingContent|.  Did you mean to use it instead of
|aContent|?

Comment 28

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

Comment on attachment 57999 [details] [diff] [review]
Patch Rev 1 (Return the correct PrimaryFrame for a BR with :before)

r=dbaron if you fix the text to use nextSiblingContent instead of
kidContent.get().  (There's no need to use .get() anymore for ==.)

Comment 29

[kinmoz]

Attachment: Patch Rev 1.1 (Address dbaron's nextSiblingContent catch)

Good catch dbaron ... I meant to use that. For the new patch I just changed:

+	       if (kidContent.get() == aContent &&

to:

+	       if (nextSiblingContent.get() == aContent &&

Comment 30

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

Comment on attachment 58573 [details] [diff] [review]
Patch Rev 1.1 (Address dbaron's nextSiblingContent catch)

r=dbaron

Comment 31

[kinmoz]

Fix checked into trunk:

    mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp  revision 1.657

Comment 32

[[not reading bugmail]]

dont know if this is the same problem as exhibited in bug 62556, when clicking
on scrollbars causes width of tables to resize and move sideways..