From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1b) Gecko/20020727 BuildID: 20020727 Hi. The MARQUEE tag has a parameter DIRECTION which can be set to LEFT or RIGHT, but also to UP or DOWN in order to do a vertical scroll (just like a film's credits), that is not implentented yet in Mozilla. You can check this at my simple webpage http://www.danielclemente.com/html/index.htm#scroll or writing the HTML code: <MARQUEE DIRECTION="UP">this is a test</MARQUEE> Thanks, Daniel Clemente Reproducible: Always Steps to Reproduce: Open an .HTM that contains this code: <MARQUEE DIRECTION="UP">this is a test</MARQUEE> Actual Results: The text is quiet. Expected Results: The text should move from bottom to top. <MARQUEE DIRECTION="UP">this is a test</MARQUEE> <MARQUEE DIRECTION="DOWN">this is a test</MARQUEE> (LEFT and RIGHT work ok, but UP and DOWN do not) <MARQUEE DIRECTION="LEFT">this is a test</MARQUEE> <MARQUEE DIRECTION="RIGHT">this is a test</MARQUEE>

Adding another sample - which seems to work fine: <marquee behavior=scroll direction=up width=130 height=80 scrollamount=1 scrolldelay=0> This works pretty well Are you sure your code was valid? </marquee>

nothing moves with attached testcase win98 2002072508

Ok, I've got it: I'm sure, <MARQUEE DIRECTION="UP">this is a test</MARQUEE> doesn't work. But this: <MARQUEE DIRECTION="up">this is a test</MARQUEE> works perfectly. So, the problem is the case.

From bug 80269: "Will <Marquee> tag be supported?" > <marquee> is a Microsoft-proprietary tag and is not in any HTML > standard, thus it is unlikely that Mozilla will support it. ==> VERIF/WONTFIX

FYI: A XBL emulation of the MARQUEE tag was checked in yesterday (bug 156979) So.. umm.. it's kinda supported now.

removing bogus dependency

Reopening. This is no longer a wontfix, it's a valid bug in a new element.

So this bug should be resummarized to "marquee attribute values must be lower-case to work", and we need to verify that case-sensitivity is a bug (HTML is case insensitive, so I expect it is). I'll leave it to Daniel to resummarize. /be

Probably it's a bug, since HTML is case-insensitive. In addition, if LEFT and RIGHT values work regardless to the case, UP and DOWN should do the same. Let me put this few examples of what works and don't work (build 2002072704) <MARQUEE DIRECTION="left">this works</MARQUEE> <MARQUEE DIRECTION="right">this works</MARQUEE> <MARQUEE DIRECTION="up">this works</MARQUEE> <MARQUEE DIRECTION="down">this works</MARQUEE> <MARQUEE DIRECTION="LEFT">this works</MARQUEE> <MARQUEE DIRECTION="RIGHT">this works</MARQUEE> <MARQUEE DIRECTION="UP">this DOESN'T work</MARQUEE> <----- FIX THIS -----> <MARQUEE DIRECTION="DOWN">this DOESN'T work</MARQUEE> <----- FIX THIS ----->

In response to comment 5: would it make sense to go back and reopen all <marquee> related bugs in the database, since we do support it now? Otherwise, we'll have more confusion duping some bugs to old WONTFIXes and some other ones to this. Hmm. I have 2002072608, and it doesn't support it yet. I'm just one day behind the events, and I'm not current enough already! Talk about bugreports from 2 month old builds. =^.^=

Attention: I spotted another bug with <MARQUEE DIRECTION="LEFT">, which moves to the right instead. See attachment.

Ok, so this is related to Bug 156979.

Will submit a patch in a few.

Please don't pull my tail too hard, it's my first try. =^.^=

Comment on attachment 93046 [details] [diff] [review] patch Index: xbl-marquee.xml =================================================================== RCS file: /cvsroot/mozilla/layout/html/document/src/xbl-marquee/resources/content/xbl-mar quee.xml,v retrieving revision 1.2 diff -u -r1.2 xbl-marquee.xml --- xbl-marquee.xml Sat Jul 26 21:07:03 2002 1.2 +++ xbl-marquee.xml Sat Jul 27 21:06:00 2002 @@ -135,7 +135,7 @@ { this.scrollStarted = 1; //we only want this to run once - switch (this.direction) + switch (this.direction.toLowerCase()) { case "up": this.dirsign = 1; @@ -151,18 +151,18 @@ this.stopAt = 0; break; - case "left": - this.dirsign = 1; - this.startAt = this.innerDiv.offsetLeft - this.outerDiv.offsetWidth; - this.stopAt = this.outerDiv.offsetWidth + this.innerDiv.offsetWidth + this.startAt; - break; - case "right": - default: this.dirsign = -1; this.stopAt = this.innerDiv.offsetLeft - this.outerDiv.offsetWidth; this.startAt = this.outerDiv.offsetWidth + this.innerDiv.offsetWidth + this.stopAt; break; + + case "left": + default: + this.dirsign = 1; + this.startAt = this.innerDiv.offsetLeft - this.outerDiv.offsetWidth; + this.stopAt = this.outerDiv.offsetWidth + this.innerDiv.offsetWidth + this.startAt; + break; } this.newPosition = this.startAt; } //end if @@ -185,7 +185,7 @@ } } - switch(this.direction) + switch(this.direction.toLowerCase()) { case "up": case "down": @@ -223,7 +223,7 @@ <![CDATA[ var height; - if ((this.direction == "up") || (this.direction == "down")) { + if ((this.direction.toLowerCase() == "up") || (this.direction.toLowerCase() == "down")) { height = "200px"; this.outerDiv.style.height = height; } @@ -238,7 +238,7 @@ this.outerDiv.style.width = this.width; - if ((this.direction == "up") || (this.direction == "down")) { + if ((this.direction.toLowerCase() == "up") || (this.direction.toLowerCase() == "down")) { this.innerDiv.style.padding = height + " 0"; }

Eep. My apologies, first one was a mess. Silly, silly me! Try rhis.

Wouldn't it be simpler to change the getter for the direction property to do toLowerCase()? (You'd still need to do the left/default combination, but not the other changes.)

Most definitely, but I'm afraid to mess something up. ;) OK, I'll try and do some serious revamping now.

Could anybody please explain me, is there any specific reason in using this: <method name="start"> if (this.scrollStarted == 0) { this.scrollStarted = 1; //we only want this to run once ...code... } instead of putting ...code... into init() method?

* Added toLowerCase() to prop retrievers * Moved a piece of code that was intended to execute only once to init() * Optimized switch()

going to steal this, I have some other issues that need to be cleaned up.

note that the reason the code was in start is that in theory, you can stop the marquee and change its direction. I just never implemented it fully, which is the bug I want to fix.

stealing bug

Doron, I doubt JS engine has builtin optimizer, so please also remove any "case"s in "switch" statement that are grouped with "default".

> cleaning up marquee xbl - adding dynamic direction changing, bgcolor > and lowercasing Should we update the summary accordingly? Are any of these problems critical to the Chinese top sites?

Don't we want to have this in RTM, if so, please nominate...

Ok, please tell me if I -the reporter- must do anything (it's my first bug). I'd like to explain another possible 'bug' of the MARQUEE tag: when we set BEHAVIOR parameter to ALTERNATE the text should bounce, but shouldn't hide itself at the borders. Check the attachment.

I am fully aware of that difference, and don't think its that important for 1.0.1. Lets try to support it as minimally as possible.

And let's keep this bug focused on the case sensitivity problem, please file new bugs for feature requests.

anyone want to review this? We'll only get qa probably once its reviewed.

this looks completely straight forward r=bstell@ix.netcom.com

Comment on attachment 93217 [details] [diff] [review] branch patch - uper case working, bgcolor, cleaned up a not needed case: + // change the scroll position + if ((this.direction == "up") || (this.direction == "down")) ... Minor style nit, fix the indentation of the comment. sr=jst

In the start(), case "left": this.dirsign = 1; this.startAt = this.innerDiv.offsetLeft - this.outerDiv.offsetWidth; this.stopAt = this.outerDiv.offsetWidth + this.innerDiv.offsetWidth + this.startAt; break; case "right": default: this.dirsign = -1; this.stopAt = this.innerDiv.offsetLeft - this.outerDiv.offsetWidth; this.startAt = this.outerDiv.offsetWidth + this.innerDiv.offsetWidth + this.stopAt; break; 1) default should be "left", not "right". 2) If "case" group contains "default" case, everything but "default" label is senseless. Therefore, case "right": this.dirsign = -1; this.stopAt = this.innerDiv.offsetLeft - this.outerDiv.offsetWidth; this.startAt = this.outerDiv.offsetWidth + this.innerDiv.offsetWidth + this.stopAt; break; default: this.dirsign = 1; this.startAt = this.innerDiv.offsetLeft - this.outerDiv.offsetWidth; this.stopAt = this.outerDiv.offsetWidth + this.innerDiv.offsetWidth + this.startAt; break; Attention, please DO NOT get "setting default to left in marquee.direction to "left"" and this "default"!!! The first one happens if marquee.direction is empty, while the second happens when it's not empty but isn't one of the keywords.

I am unclear how this is related to making the marquee code case insensitive. This sounds like a separate issue. Would you please open a separate bug?

It's more a matter of code optimizaton than a real bug. Besides, only a couple trivial changes needed to fix it, so why starting the process from the very beginning when it's possible to add a couple lines to current patch and resolve it once and forever?

In general the idea of getting multiple things in at once is good but time is very tight right now and getting an approval is harder the larger the patch is.

Comment on attachment 93217 [details] [diff] [review] branch patch - uper case working, bgcolor, cleaned up a not needed case: a=chofmann for 1.0.1 on this update to the patch for the branch.

checked into the branch

posthumus adt1.0.1+. brian: pls do not check in to the 1.0 branch, without ADT approval. ADT approval is expressed with a "adt1.0.1+" in Keywords, and a comment from the ADT that the patch is approved for checking into the branch. thanks!

Sorry, I thought a=chofmann was approval.

brian: a=chofmann is drivers' aproval for the branch. netscape employees checking into the branch are also required to have ADT approval before checking into the 1.0 branch. additionally, once you have checked something into the 1.0 branch, you should add the keyword "fixed1.0.1", so that QA is notified to verify the fix. thanks! ji: xianlan, can you pls verify this as fixed on the branch, then replace "fixed1.0.1" with "verified1.0.1" doron: will you or brian be checking this into the trunk?

jaime: it's quite ironic that you've called brian on this point given that the original marquee for 1.0.1 landed a=jaime w/o any driver approval, especially not for 1_0 which is supposed to get features *after* trunk.

I'll be glad to check it into the trunk once I know it has approval.

ji - can you verify this bug fix in branch? When verified, pls replace fixed1.0.1 keyword with verified1.0.1. Thanks.

2002082304/WinXP - testcase still doesnt work.

> ji - can you verify this bug fix in branch? When verified, pls replace > fixed1.0.1 keyword with verified1.0.1. Thanks. ji is on vacation until 8/27. Carine can you help? or Andreas is there someone else that can?

verified on 1.0.1 branch

Should someone check-in the patch into the trunk, or first the bug must be marked as RESOLVED ? With Linux 2002100604, the bug is present yet.

This bug is still present in Mozilla 1.4. I also note that some HTML examples sites (such as http://www.htmlcodetutorial.com/_MARQUEE_DIRECTION.html) don't use any lower case for their HTML examples.

I concur with the analysis. In addition, the "alternate" behavior NEVER works (it should bounce from side to side; instead it slides across like a normal marquee). <marquee width=100% behavior=alternate>test</marquee> That code bounces on the right side, but slides out on the left before returning to the right. Kinda weird, if you ask me. Mozilla 1.5, Win32, build 20030925.

lower-case is now fixed

Fixed by what? ->WORKSFORME

Doh! My apologies. The patch in this very bug. (Slapping myself on the face and resetting.)