160162 - MARQUEE loop property not supported

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Erik Arvidsson]

The loop property/attribute for the MARQUEE XBL emulation is not supported.

Comment 1

[Matthias Versen [:Matti]]

-> doron (marquee in mozilla sucks)

Comment 2

[Doron Rosenberg (IBM)]

wontfix, it isn't a 100% emulation

Comment 3

[Erik Arvidsson]

My own XBL emulation supports this. Feel free to reuse the code from that. See 
http://webfx.eae.net/dhtml/xblmarquee/xblmarquee.html

To me it is pretty odd that you reimplemented what I already had done and you 
did not even improve on the existing emulation. I know my version has bugs but 
at least it is a lot less buggy than the one you checked into the tree.

Comment 4

[José Jeria]

WONTFIX?
What is the point of supporting the Marquee tag if you aren't going to support 
it 100%? You now decided to support it because many sites use it. We all know 
its not a W3c standard, but what is the point of supporting a MS extension only 
to a certain level? If it is not 100% just remove it totally.

Comment 5

[Mats Palmgren (inactive)]

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

Comment 6

[Marek]

I believe the LOOP attribute (and the related OnFinish event) should be
implemented in Mozilla's MARQUEE tag implementation. My usage of the feature,
and justification for it to be supported, is on a web site that uses a narrow
area to display system notification messages (like success or error messages on
a web application after submitting a form). My web app (unfortunately I cannot
share the URL because it is a confidential project) uses frames, one of which
hosts a sort of status bar implemented with a MARQUEE. After the message scrolls
all the way, it triggers the OnFinish message which causes the page to clean up.
A usage example is reported in Bug 166857.

Comment 7

[Doron Rosenberg (IBM)]

be a real developer, use javascript for just one loop.  It really isn't that
hard to write a js marquee (I've written 3).

Comment 8

[Marek]

Well of course, but that's not the point. There are tons of web pages out there,
many of which have been implemented for a long time with IE's HTML specs in mind
(like MARQUEE), mine included. What I would like to see is Mozilla catching up
with the feature set offered by IE and to which the vast majority of web sites
already conform. I totally agree with José Jeria's opinion of "What is the point
of supporting the Marquee tag if you aren't going to support it 100%"

Please consider implementing the MARQUEE tag to 100% or removing the tag
altogether. I'm sure it's not that hard to implement what's missing. You even
have Erik Arvidsson's code to use.

Thanks!

Comment 9

[Doron Rosenberg (IBM)]

>Please consider implementing the MARQUEE tag to 100% or removing the tag
>altogether. I'm sure it's not that hard to implement what's missing. You even
>have Erik Arvidsson's code to use.

first lemme remove css support, since its not 100%...

Netscape implemented marquee to fulfill a certain need, not to be fully
complaint. Indeed, we would have to allow IE4-style rules to work for that.

And Erik is free to provide a patch.

Comment 10

[Mats Palmgren (inactive)]

I have a patch for this if you want to add it...

Comment 11

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 12

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 13

[Doron Rosenberg (IBM)]

Doesn't loop execute some code once its done? Don't see that in this patch

Comment 14

[Martijn Wargers (dead)]

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

Comment 15

[Martijn Wargers (dead)]

http://msdn.microsoft.com/workshop/author/dhtml/reference/objects/marquee.asp

onstart event:
http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onstart.asp

onfinish event:
http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onfinish.asp

onbounce event:
http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbounce.asp

These are the marquee specific events that I know of. Neither of them are
implemented in Mozilla.

Comment 16

[José Jeria]

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

Comment 17

[Martijn Wargers (dead)]

Attachment: testcase2

Comment 18

[Martijn Wargers (dead)]

Attachment: testcase3

Comment 19

[Martijn Wargers (dead)]

Attachment: patch1

Ok, this patch makes Mozilla behave almost like IE6.
It also fixes bug 315575.

Comment 20

[Martijn Wargers (dead)]

The onstart, onfinish and onbounce events I would like to implement in a different bug, if you don't mind.

Comment 21

[Doron Rosenberg (IBM)]

Comment on attachment 205978 [details] [diff] [review]
patch1

File convention is that the <![CDATA is indented on the same level as the inner HTML.

> 
>+      <property name="loop">
>+        <getter>
>+        <![CDATA[
>+	        var val = parseInt(this.getAttribute('loop'));
>+	  
>+          if (val < -1 || isNaN(val))
>+	          return this._loop;
>+
>+	        return val;
>+	      ]]>

there is some weird indenting going on here.  Tabs perhaps?

> 
>+      <method name="_set_loop">
>+        <parameter name="aValue"/>
>+        <body>
>+        <![CDATA[
>+          if (!aValue || isNaN(aValue))
>+            return false;
>+
>+          if (aValue < -1)
>+            aValue = -1;
>+
>+          this._loop = aValue;
>+          return true;
>+        ]]>
>+        </body>
>+      </method>

Call it _setloop.  Same story about cdata indentation.

>@@ -318,24 +352,43 @@
> 
>                 // swap direction
>                 const swap = {left: "right", down: "up", up: "down", right: "left"};
>                 this._direction = swap[this._direction];
>                 this.newPosition = this.stopAt + (this.dirsign * this._scrollAmount);
>               break;
> 
>               case 'slide':
>-                if (this.newPosition == this.stopAt)
>-                  return;
>-                this.newPosition = this.stopAt;
>+                if (this._loop > 1) {
>+                  this.newPosition = this.startAt;
>+                }
>+                else {
>+ 	                if ((this._direction == "up") || (this._direction == "down"))
>+                    this.outerDiv.scrollTop = this.stopAt;
>+                  else
>+                    this.outerDiv.scrollLeft = this.stopAt;
>+                  this.stop();
>+                  return;                  
>+                }

weird indentation, tabs?

>               break;
> 
>               default:
>                 this.newPosition = this.startAt;
>             }
>+
>+            if (this._loop > 1)
>+ 	            this._loop--;
>+ 	          else if (this._loop == 1) {
>+ 	            if ((this._direction == "up") || (this._direction == "down"))
>+                this.outerDiv.scrollTop = this.stopAt;
>+              else
>+                this.outerDiv.scrollLeft = this.stopAt;
>+ 	            this.stop();
>+ 	            return;
>+ 	          }

weird indentation again.

Comment 22

[Martijn Wargers (dead)]

Attachment: patch2

Yes, somehow tabs crept into the patch. This should fix it and the cdata indenting.

Comment 23

[Martijn Wargers (dead)]

(In reply to comment #21)
> Call it _setloop.  Same story about cdata indentation.
I'd rather have it named _set_loop, since I change the _loop value, not the loop value. Also, that is consistent with the other methods, _set_behavior, _set_direction, etc.
Let me know if you agree/disagree.

Comment 24

[Johnny Stenback (:jst)]

Comment on attachment 206198 [details] [diff] [review]
patch2

sr=jst

Comment 25

[:Gavin Sharp [email: [e-mail]]]

mozilla/layout/style/xbl-marquee/xbl-marquee.xml; new revision: 1.22;

Comment 26

[Martijn Wargers (dead)]

I filed bug 324408 for the onstart, onfinish and onbounce events stuff.