441703 - direction attribute of marquee should be case-insensitive (for selector matching)

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Thom Roberts]

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

When viewing my website, the text in the dedication box which scrolls up is not in the box as it's supposed to be.  Instead it is higher up on the page, scrolling over other text.  This only occurs in Firefox 3, not Firefox 2.

Example:
http://www.retroradionetwork.com/thoms/RR-FF3.png

Reproducible: Always

Steps to Reproduce:
1. Start Firefox 3
2. Navigate to http://www.renegaderetro.com
Actual Results:  
When the page loads, the dedication text is scrolling near the top of the page, on top of a menu rather than in its box.

Expected Results:  
The text should be scrolling within its block.

Example:
http://www.retroradionetwork.com/thoms/RR-Opera.png

The software should display the scrolling text in the block that is supposed to contain it.

I have tested the site in Firefox 3, Firefox 2, Opera 9.50, Opera 9.24, Internet Explorer 7 and Internet Explorer 6.  I have tried it on 3 different PC's.

Firefox 3 is the only browser which displays this behavior, and does it on all PC's.  It occurs in both Windows and Linux.  I am enclosing the code of the dedication block in case that is the problem/the problem can be found using it.


[code]
<?php

if (eregi("DedicationScroller",$_SERVER['PHP_SELF'])) {
    Header("Location: index.php");
    die();
}

include("includes/sam-config.php");

$totalnum = "10"; //The number of dedications to show on the site

  $SAMdb->open("SELECT songlist.artist, songlist.title, requestlist.name, requestlist.msg FROM songlist, requestlist WHERE requestlist.name != '' and requestlist.songid = songlist.id and requestlist.msg != '' ORDER BY requestlist.t_stamp DESC LIMIT $totalnum");
  $myrow = $SAMdb->rows();
  reset($myrow);

 $content .= "<marquee behavior='scroll' direction= 'Up' width= '130'
height= '190' scrollAmount= '2' onMouseover='this.scrollAmount=1'
onMouseout='this.scrollAmount=2' onMouseDown='this.scrollAmount=0'
onMouseUp='this.scrollAmount=1'>\n";
 $content .= "<table><tr><td>";

foreach($myrow as $row) {

 $content .= "<b>Artist:</b><br>";
 $content .= $row['artist'];
 $content .= "<br><br>";
 $content .= "<b>Song Played:</b><br>";
 $content .= $row['title'];
 $content .= "<br><br>";
 $content .= "<b>Requested by:</b><br>";
 $content .= $row['name'];
 $content .= "<br><br>";
 $content .= "<b>Dedication:</b><br>";
 $content .= stripslashes($row['msg']);
 $content .= "<hr>\n";
}
 
 $content .= "</td></tr></table>";
 $content .= "</marquee>\n";
?>
[/code]

Comment 1

[Ria Klaassen (not reading all bugmail)]

Regression range 1 (no text anymore in Dedication box):
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1184103420&maxdate=1184111159
-> Bug 357614.

Regression range 2 (text reappeared but on the wrong place):
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1199875440&maxdate=1199885279
-> Bug 407016.

Comment 2

[philippe (part-time)]

Given that the whole scrolling object is a marquee, it is more likely to be a regression from bug 407016.

Cc'ing Martijn.

Comment 3

[Martijn Wargers (dead)]

Attachment: patch

Thanks for the regression range.
This is indeed a regression from bug 357614. I already wondered about this in bug 425247, which is basically about the same issue.
It seems to me that the direction attribute should be added to the list. Or else, I should update the patch in bug 425247 with Up, uP, Down, DOwn, DOWn, dOwn, dOWn, dOWN, doWn, doWN, dowN values (which seems like a bad idea to me).

Comment 4

[Martijn Wargers (dead)]

There is a testcase in bug 425247.

Comment 5

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

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

>Index: layout/style/nsCSSParser.cpp

>               "type",
>+              "direction",
>               nsnull

r+sr=dbaron if you add an additional comment above your added line saying something like:
  // additional attributes not in HTML4
and then after "direction" have a comment saying "// marquee".

Comment 6

[Martijn Wargers (dead)]

Attachment: updated patch

Carrying over r+/sr+

Comment 8

[Reed Loden [:reed]]

Pushed in 15871:b9344d23a2fc.

Comment 9

[Martijn Wargers (dead)]

Comment on attachment 327581 [details] [diff] [review]
updated patch

Very safe fix for this regression.
This was also noticed in bug 446111 and will likely cause more dupes if it doesn't get fixed.

Comment 10

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

I agree that we should take this in 1.9.0.*.

Comment 11

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

Can we get a test for this bug?

Comment 12

[Martijn Wargers (dead)]

Sorry for the delay, there was a reftest attached to the patch in the duped bug 425247, but maybe that's the wrong test for this?
In that case, I guess direction should be added to http://mxr.mozilla.org/mozilla-central/source/layout/style/test/test_bug357614.html?raw=1

Comment 13

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

Martijn, it's not clear to me if that's the right test or not.

dbaron, thoughts?

Comment 14

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

Either or both is ok.

Comment 15

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

Comment on attachment 327581 [details] [diff] [review]
updated patch

Approved for 1.9.0.2. Please land in CVS. a=ss

Please be sure to land the reftest as well (both on mozilla-central and on 1.9).

Comment 16

[Martijn Wargers (dead)]

Attachment: trunk diff of reftests from bug 425247

Comment 17

[Martijn Wargers (dead)]

It turns out I don't have an hg account yet, so I can't check these reftests in trunk yet.
Maybe someone else could check them into trunk?

I need them to be checked into trunk, so I can check the patch and the reftests into the 1.9 branch.

Comment 18

[Martijn Wargers (dead)]

Ok, I actually do have a hg account, I didn't have it correctly set up. But I do now, and I checked in the reftests on trunk now:
http://hg.mozilla.org/index.cgi/mozilla-central/rev/8410e5981323

Comment 19

[Martijn Wargers (dead)]

RCS file: /cvsroot/mozilla/layout/reftests/bugs/425247-1.html,v
done
Checking in layout/reftests/bugs/425247-1.html;
/cvsroot/mozilla/layout/reftests/bugs/425247-1.html,v  <--  425247-1.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/425247-1-ref.html,v
done
Checking in layout/reftests/bugs/425247-1-ref.html;
/cvsroot/mozilla/layout/reftests/bugs/425247-1-ref.html,v  <--  425247-1-ref.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/425247-2.html,v
done
Checking in layout/reftests/bugs/425247-2.html;
/cvsroot/mozilla/layout/reftests/bugs/425247-2.html,v  <--  425247-2.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/425247-2-ref.html,v
done
Checking in layout/reftests/bugs/425247-2-ref.html;
/cvsroot/mozilla/layout/reftests/bugs/425247-2-ref.html,v  <--  425247-2-ref.html
initial revision: 1.1
done
Checking in layout/reftests/bugs/reftest.list;
/cvsroot/mozilla/layout/reftests/bugs/reftest.list,v  <--  reftest.list
new revision: 1.460; previous revision: 1.459
done
Checking in layout/style/nsCSSParser.cpp;
/cvsroot/mozilla/layout/style/nsCSSParser.cpp,v  <--  nsCSSParser.cpp
new revision: 3.387; previous revision: 3.386
done

Checked into the 1.9.0.x tree.

Comment 20

[Martijn Wargers (dead)]

To verify, there was a testcase attached in the duped bug 425247: https://bugzilla.mozilla.org/attachment.cgi?id=311857

Comment 21

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2) Gecko/2008090212 Firefox/3.0.2.