376184 - ignore margin-top of <p> inside <marquee>

Status: NEW

(Core :: Layout, defect)

Description

[j.j.]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070307 Minefield/3.0a3pre
Build Identifier: 

Block elements with margin inside a marquee element behave different in IE.

IE is inconsistent and handles <div> different from <p>, see testcase.
(<p> and it's margins are a special thing in IE anyway)

For more compatibility with IE I'd propose to ignore top margin of block elements inside marquee.
This would "fix" at least #1, #2, #4 and (partial)#7 of testcase.

Reproducible: Always

Comment 1

[j.j.]

Attachment: testcase1

Comment 2

[j.j.]

btw., #1 and #4 show the same problem as bug 314445.

#1 an #4 regressed against FF2.

Comment 3

[Martijn Wargers (dead)]

So when looking at your testcase, I observe 2 things for IE:
- the default margin of <p> inside marquee is 0 (alos in standards mode)
- The margin-top value of elements in vertical marquees is ignored.

So I think those are 2 things that Mozilla also could be doing to get more compatible marquees.

The testcase also shows a bug where margin-bottoms aren't rendered for elements inside marquees, that has probably something to do with the use of display: -moz-box.

Also, the background colors for #2, #3 and #5 are 100% wide for IE, while it is small for Mozilla. Not sure what can be done about that.

Comment 4

[Martijn Wargers (dead)]

Attachment: testcase2

This testcase shows that IE ignores:
- the margin-top when <p> is the first node inside the marquee
- the margin-bottom when <p> is the last node inside the marquee

Comment 5

[Martijn Wargers (dead)]

Attachment: testcase3

The same goes for vertical marquees.

Comment 6

[Martijn Wargers (dead)]

Attachment: testcase4

This shows that margin-top seems to be always ignored for the elements which are the first node in vertical marquees (with behavior=alternate?).

Comment 7

[Martijn Wargers (dead)]

Attachment: testcase5

..which is not the case for horizontal marquees.

Comment 8

[j.j.]

I propose the following addition in html.css:

 marquee[direction=up] :first-child, marquee[direction=down] :first-child {
   margin-top: 0 !important; 
 }
 marquee p:first-child {
   margin-top: 0;
 }

This fixes all of testcase1, besides the background width issue and the margin-bottom thing of #3, #6 and #7 (which imo is less important).

(ps. Aren't your testcases wrong?)
 #1, #1a, 

Comment 9

[j.j.]

We have other block-like elements than p with margins (pre, ol, ul...). So this is better:

 marquee[direction=up] :first-child, marquee[direction=down] :first-child {
   margin-top: 0 !important; 
 }
 marquee :first-child {
   margin-top: 0;
 }

In IE it is impossible to override top margins in the up/down case, so we need !important here.
In the general case you can override the default margin behaviour for <p>, <pre>, .... 

Comment 10

[j.j.]

this order I like more...

 marquee :first-child {
   margin-top: 0;
 }
 marquee[direction="up"] :first-child, marquee[direction="down"] :first-child {
   margin-top: 0 !important; 
 }

Comment 11

[Martijn Wargers (dead)]

Ok, I guess it needs similar treatment as:
http://lxr.mozilla.org/seamonkey/source/layout/style/quirk.css#127

I think any new rules specific to marquee should reside here:
http://lxr.mozilla.org/seamonkey/source/layout/style/xbl-marquee/xbl-marquee.css

>In IE it is impossible to override top margins in the up/down case, so we need
>!important here.

Actually, I suspect that the computed style is still there for the top margin of the element, but that the vertical marquee just 'ignores' it in IE.

>(ps. Aren't your testcases wrong?)
> #1, #1a,

Could you explain? They could very certainly be wrong. 

Comment 12

[j.j.]

(In reply to comment #11)
> Ok, I guess it needs similar treatment as:
> http://lxr.mozilla.org/seamonkey/source/layout/style/quirk.css#127

Yes, even though I don't know what :-moz-first-node exactly does, I'd say:

  marquee > :-moz-first-node {
   margin-top: 0;
 }
 marquee[direction="up"] > :-moz-first-node, marquee[direction="down"] > :-moz-first-node {
   margin-top: 0 !important; 
 }
We shouldn't need the optimisation for marquee (listing all possible elements with top margin). Typically marquee hasn't lots of child nodes.


> I think any new rules specific to marquee should reside here:
> http://lxr.mozilla.org/seamonkey/source/layout/style/xbl-marquee/xbl-marquee.css

(I never saw this file)

> >In IE it is impossible to override top margins in the up/down case, so we need
> >!important here.
> 
> Actually, I suspect that the computed style is still there for the top margin
> of the element, but that the vertical marquee just 'ignores' it in IE.

We should not care much about, should we? A simple CSS patch does the job.

> >(ps. Aren't your testcases wrong?)
> > #1, #1a,
> 
> Could you explain? They could very certainly be wrong.

My time is short today (and was yesterday), I'll look again tomorrow.
(ignore |#1, #1a,| this was a typo) 

Comment 13

[j.j.]

Attachment: testcase6 with propsed CSS

(In reply to comment #11)

> Could you explain? They could very certainly be wrong. 

Testcase2 is wrong or I can't understand it.

Anyway, I created a new testcase. You can add dynamicly the style rules proposed in comment 12. I think that's the way to go.
It fixes:
 * horizontal case (all top-margin issues)
 * case "up-slide" (completely)
 * case "up-alternate", case "down-alternate", case "down-slide" (all top-margin issues and some bottom issues)
 Note: The general up/down case had no problems (besides wrong start position, maybe).

Comment 14

[Martijn Wargers (dead)]

Testcase2 is the show the difference when the <p> is the first/last node in the marquee or not.
I just added an explanation for :-moz-first-node here:
http://developer.mozilla.org/en/docs/CSS::-moz-first-node
With some text at the start of the marquee, the first <p> would still match  the :first-child selector, which it doesn't for the :-moz-first-node selector.

Thanks for that new testcase.

Comment 15

[Martijn Wargers (dead)]

Attachment: testcase7

The computed style is indeed still there, it becomes relevant when the <div> would be absolutely positioned. But it's probably too far fetched to care about that.
So I think you're right margin: 0 !important; might indeed be the best solution.

Comment 16

[j.j.]

> I just added an explanation for :-moz-first-node here:
> http://developer.mozilla.org/en/docs/CSS::-moz-first-node

Cool, give us more of this... 
(even though this case was almost obviously, besides the whitespace thing)

> margin: 0 !important; might indeed be the best solution.

So go ahead, make a patch and ask for review!

Comment 17

[Martijn Wargers (dead)]

Attachment: patch

Comment 18

[Martijn Wargers (dead)]

Comment on attachment 261021 [details] [diff] [review]
patch

I suspect you don't like adding additional rules in html.css for this.

I'm not sure how to get it working otherwise, other than a rather hacky way.

Comment 19

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

All the existing rules like this are in quirk.css, for what it's worth.

Comment 20

[j.j.]

> All the existing rules like this are in quirk.css, for what it's worth.

Reverse-engineering MS's marquee completely only for quirks mode would be a bad idea.

Comment 21

[Martijn Wargers (dead)]

dbaron was talking about the margin-top: 0; rule only.

Comment 22

[j.j.]

Yes, I tryed to say leaving this rules to quirks.css results in incomplete reverse-engineering of marquee.
This makes not much sense, given increasing ammount of webpages using strict doctype without decreasing use of marquee.

Comment 23

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

Comment on attachment 261021 [details] [diff] [review]
patch

I think this should be done the same way we do it for other things if we do it at all.  That said, our rules in quirks.css are pretty horrible and we should really fix bug 33784.  I'm not sure I'd want the performance hit of fixing this at all, given the frequency of use of marquee.  And the rule you've added without an element name is going to be very slow.  That's for the first rule you're adding.

I'd think the second rule you're adding could be fixed in the marquee code by having something that margins collapse through inside the marquee XBL binding, and just scrolling the marquee from one border edge of it to the other.

Comment 24

[Martijn Wargers (dead)]

David, thanks for the review (and explanation).
I agree with you, this is a rather trivial bug and if it would slow things down, it shouldn't be used.
I guess this could also be solved in a different way if styleexplicitcontent would work for xbl.