234675 - [FIXr]flash movie is cut in 1.5 and 1.6

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Radoslav Bielik]

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113

The page at http://dev.neomyz.com/skindemo.aspx show a single flash movie 
which can automatically change it's size based on the length of its content 
(it is a web poll). Mozilla 1.5 and newer displays such a flash movie 
incorrectly, causing it to offset itself inside it's rectangle which in the 
end causes it to appear cut on the top, or on the bottom (this changes with 
different polls). I did test this with Mozilla 1.2.1 and 1.4 and it worked 
fine, as it does with all other major browsers (Internet Explorer, Netscape 
4.7 and 7.1, Opera 7.11, AOL 8.0, etc). It is only displayed incorrectly in 
Mozilla 1.5, 1.6.0.2004011308, and I have also tried the latest nightly build 
1.7.0.2004021608 and it is displayed the same way (incorrectly).

I have searched the older bugs and found a similar bug report at 
http://bugzilla.mozilla.org/show_bug.cgi?id=215497, but I am not sure if this 
is the same bug, although it seems to be, but the abovementioned bug report is 
about 6 months old and appears to be abandoned. I consider this to be a major 
bug and hope that it can be fixed in a reasonable time as it was working fine 
in previous versions of your browser (which I think has grown to be a great 
browser, btw).

I can get in contact with our flash developer and provide you with details on 
how the internal size and positioning in the poll flash movies is calculated, 
I think that this could perhaps help us find out the source of this problem. 
PPLease let me know what should I do next / what do you think about this bug. 
Thanks in advance!

Best regards,
Radoslav Bielik

Reproducible: Always
Steps to Reproduce:
The following lists some websites that are using our web poll. You can visit 
any of them to reproduce this bug. They contain various instances/skins of the 
web poll, which means that each one is cut in a different way:

http://www.neomyz.com - web poll service provider
http://www.topshop.sk/topshop/script/novinky.asp?kat=0&vyrobok=0
http://www.atstracts.org
http://4cc.mosqito.net/
http://www.hollywoodvideo.com/movies/features/awards/04/awardspoll.aspx

Actual Results:  
A flash movie is displayed incorrectly

Expected Results:  
A movie is supposed to display correctly (uncut), try Internet Explorer 5 or 
newer, or Mozilla 1.4 or older ;)

Comment 1

[Radoslav Bielik]

Attachment: Screenshot showing IE, Opera, and Mozilla comparison

This screenshot show Internet Explorer 6, Opera 7.11 (these two shows how the
flash poll is displayed correctly) and Mozilla 1.6 (this one displays the flash
poll moved too far to the top and thus cut).

Comment 2

[Boris Zbarsky [:bzbarsky]]

At http://dev.neomyz.com/skindemo.aspx the <embed> in question is set to a
height of 231, no?  That's the height I'm seeing it here....

Is the plugin itself supposed to resize the <embed>?  Or how does the auto
sizing work?

Comment 3

[Radoslav Bielik]

(In reply to comment #2)
> At http://dev.neomyz.com/skindemo.aspx the <embed> in question is set to a
> height of 231, no?  That's the height I'm seeing it here....
> Is the plugin itself supposed to resize the <embed>?  Or how does the auto
> sizing work?

No, the "height" attribute of the <embed> tag is always calculated on the 
server side in the ASP.NET code which means that the resulting HTML code has 
always correct value of this, it is 231 pixels in case of this test poll.

But since the size of the <embed> changes, the flash movie needs to position 
itself inside its rectangle, this is done in the ActionScript code in the 
flash movie. Perhaps it doesn't receive the correct values on which the 
calculation depends from the browser? I don't know the details of this, but I 
will ask our flash developer how this is done and post details here tomorrow 
(thursday). I will also ask him to create some kind of test movie which will 
show what exactly is causing the wrong positioning. 

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, more information on exactly what you guys are doing would be great.... 
231px is far too short to contain the entire movie in this case....

Comment 5

[Radoslav Bielik]

(In reply to comment #4)
> Yeah, more information on exactly what you guys are doing would be great.... 
> 231px is far too short to contain the entire movie in this case....

231px is the exact height of the full movie (otherwise it would be displayed 
cut in other browsers too). I have trimmed the screenshot of the IE showing 
the poll, and it's height is 230px (I probably cut it an extra pixel of height 
accidentally), so the height is calculated and provided correctly in the HTML 
code. 

I will attach this screenshot for you to see, but unfortunately our flash 
developer is now sick, I hope he will be back in the office on thursday to 
discuss the resizing/positioning details with him.

Comment 6

[Boris Zbarsky [:bzbarsky]]

All good.  Just have him comment on this bug, please.

Comment 7

[Radoslav Bielik]

Attachment: Correct appearance

This screenshot shows the correct appearance of the flash movie, and its
approximate size (I have just cut it in MS Paint so the size of this picture is
150x230, and the size of the movie itself is 148x231).

Comment 8

[Radoslav Bielik]

I'm sorry for the delay but the Flash guy is still sick, so I won't be able to 
post further details before the next week. I will get back on Monday or 
Tuesday, and I hope that this problem will be solvable without too much hassle 
as soon as we have more information... :)

Comment 9

[Radoslav Bielik]

Hi,

I think that we have identified the possible cause of this bug (well, I'm not 
100% sure, but say 95% :). Mozilla 1.5 or higher just ignores the "salign" 
attribute of the <embed> tag.

I have created a set of test files for you, and I will post them shortly as an 
attachment. This set contains a simple flash movie with source (both swf and 
fla files) which contains only a 200x200px green rectangle and nothing else.

There are two HTML pages with this flash movie embedded surrounded by <hr> on 
top and bottom - test_center.html and test_top.html. The embed tag in both of 
them has the scale="noscale" attribute, which means that if the width/height 
attributes of the <embed> tag are different than the internal size of the 
flash movie, if won't be scaled/streched, but it's original size will be 
preserved. The width of the <embed> is 200px and height is 300px in both HTML 
files. 

In the test_center.html, the "salign" attribute is omitted, which results in 
the 200x200px flash movie being vertically centered in its 200x300px 
rectangle. The test_top.html contains the salign="t" attribute, which SHOULD 
result in the flash movie being vertically aligned to top of its 200x300px 
rectangle. The screenshots test_center_correct.jpg and test_center_top.jpg 
show the correct appearance and both were made with Mozilla 1.4, which is the 
last version to work correctly. 

If you test the HTML files in Mozilla 1.4, IE6.0 or Opera 7.11, you will get 
the correct appearance, however Mozilla 1.6 seems to ignore the salign 
attribute.

Please let me know if you have enough information on thig bug or if you need 
further clarification. Thanks in advance!

Comment 10

[Radoslav Bielik]

Attachment: This is a simple test case.

This is a simple test case containing a flash movie with source, and two HTML
test pages + screenshots showing correct appearance. It demonstrates incorrect
handling of the salign attribute by Mozilla 1.6.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Hmm... salign is handled completely by the plugin, is it not?

Having the testcase, helps a lot, yes.  ;) The behavior on the test_top testcase
changed between build 2003-07-21-05 and build 2003-07-22-22.  The checkins in
that range are:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2003-07-21+05%3A00%3A00&maxdate=2003-07-22+22%3A00%3A00&cvsroot=%2Fcvsroot

And in fact, the specific checkin involved was the one to the HTML content sink.
 The upshot of that was that we started storing attributes in a different order
(and giving them to the plugin in a different order as a result).  And the
particular testcase you posted breaks in a 1.4 build if I put the "salign"
attribute of the <embed> anywhere before the "scale" attribute.

So we have the following questions:

1)  Should the behavior of the plugin depend on the order of the attributes on
    the <embed>?  (I would say "no", but what do I know about plugins?  ;) ).
    I'm not sure what the plugin API promises and whether salign is handled by
    the flash plugin itself of by the movie running in it.
2)  Should we fix things so that the attributes are given to the plugin in the
    old order?   (I'd say probably yes, even if the answer to the first question
    is "no").

sicking, just ccing you so you'll be aware of this issue... :(

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch for issue #2

This makes us pass the attributes to the plugin in the same order as we used
to. It fixes all the testcases pointed to in this bug.	At the same time, this
is a pretty nasty hack.  I don't like it one bit.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Peter, jst, sicking, peterv, any thoughts?

Comment 14

[Radoslav Bielik]

Hi Boris and thank you for a quick and detailed response!

(In reply to comment #11)
> So we have the following questions:
> 1)  Should the behavior of the plugin depend on the order of the attributes 
on
>     the <embed>?  (I would say "no", but what do I know about plugins?  ;) ).
>     I'm not sure what the plugin API promises and whether salign is handled 
by
>     the flash plugin itself of by the movie running in it.

I think that the attribute is handled by the plugin, not by the movie itself, 
as I'm not aware if it is available to the movie (the test movie submitted 
above contains no scripting at all, only a green square). But as you say 
above, I would also say "no" - the order of attributes shouldn't be important 
to the plugin, although now it seems that the flash player plugin ignores the 
salign attribute until it doesn't find the scale attribute (and I would blame 
the flash plugin for incorrect handling of attributes).

> 2)  Should we fix things so that the attributes are given to the plugin in 
the
>     old order?   (I'd say probably yes, even if the answer to the first 
question
>     is "no").

I would also say "yes", as there might be other flash movies or even other 
plugins that depend on this. I have tried your test (swapping the salign and 
scale attributes) and I was able to "break" Mozilla 1.4 as well as Opera 7.1, 
which means that opera passes the attributes in the same order as they appear 
in the file. I was also trying this with the <param ...> tags in the <object> 
tag but it was still working correctly in the Internet Explorer, but I suppose 
that this has something to do with different version of the Flash Plugin used 
there. Perhaps it would be a good idea to notify Macromedia of this problem?

Would you please let me know what will be the next steps regarding this issue? 
Will you pass the attributes to the plugin in the "old" way (in the order in 
which they appear in the file)? Or will there be a different solution (and in 
which version)?

Thanks again for being so helpful and quick.
Radoslav Bielik

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

um... why does it matter if the doc is XML or HTML? Why wouldn't plugins have
problems with wrong attribute order in XML documents?

>At the same time, this is a pretty nasty hack.

like the rest of that file? ;)

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This makes me very worried. Noone should depend on order of attributes,
especially not such major plugins as the flash-plugin. I'd really like to hear
from some plugin people before this was checked in to the tree.

Is there any way that it could be this particular flash-movie that does
something "wrong" rather then the plugin itself? Seems strange that we wouldn't
hear more about this if salign got hosed this long ago.

How come IE is able to be unsensitive to attribute order. Could it be that they
always pass attributes to the plugin in some certain order, independent of the
order in the DOM/markup? If so, could we do the same?

Just chaning to sort in reversed order doesn't seem like a good fix since it'll
"break" as many testcases as it'll fix. It just happens that the one attached in
this bug belongs to the group that gets fixed.

Comment 17

[Boris Zbarsky [:bzbarsky]]

OK.  In order:

1)  If the plugin binary itself is what's broken (rather than the movie) then we
    have to fix the attribute order back to what it was before.
2)  HTML vs XML matters because attribute storage order is different.  See the
    AddAttributes methods in the two content sinks.
3)  We've run into issues with the Flash plugin being sensitive to order of
    things in the past, now that I think about it.  So it's not too surprising.
4)  IE uses a different plugin binary and a different plugin API (the ActiveX     
    plugin API instead of the NPAPI).  Therefore, this working in IE could be an
    API difference, or just it being a totally different flash plugin codebase.
5)  Any testcases that "break" with this fix are already broken in Opera, NS4,
    Konqueror, Mozilla 1.4 and earlier, and any other browsers that use NPAPI.
    Since the only reason to use that <embed> is to have it work with those
    browsers, I doubt we'll break anything, really.
6)  salign doesn't seem to be all that widely used...  And the green square
    movie really can't be doing anything wrong that I can see.

I'll make sure to ask a plugins person for review.  ;)

Radoslav, the next steps are us agreeing what to do and then checking in a
patch, probably.  Hopefully for 1.7b.

Comment 18

[Radoslav Bielik]

Hi Jonas,

(In reply to comment #16)
> Is there any way that it could be this particular flash-movie that does
> something "wrong" rather then the plugin itself? Seems strange that we 
> wouldn't
> hear more about this if salign got hosed this long ago.

Thee flash-movie in the testcase is provided with the source and anyone can 
check that it does nothing else but draw a green rectangle. In the flash 
publish settings for HTML page, one can choose the alignment, and it does 
nothing else but presets the salign attribute of the <embed> tag in the 
generated HTML. I don't think it could be considered that there's something 
wrong in this particular trivial flash-movie, while it is working as expected 
in majority of the browsers. But on the other hand, I can understand that 
you're concerned about this, as I don't think that the plugin should rely on 
the order of the attributes. It is also possible that the salign attribute is 
not used widely, as it is usually not needed (most of the flash-movies I've 
seen are using the same width/height of the embed tag as their internal 
height). The salign attributes is useful only with scale="noscale" when you 
dynamically resize your flash movie (using a server side script) and want the 
content of the movie to adapt to it's new size, instead of stretching/scaling. 
I wouldn't find about this issue myself if a friend of mine who is using the 
poll did not point to me that it is trimmed in Mozilla 1.5 (I did all the 
testing with Mozilla 1.4)


> How come IE is able to be unsensitive to attribute order. Could it be that 
> they
> always pass attributes to the plugin in some certain order, independent of 
> the
> order in the DOM/markup? If so, could we do the same?

I think that this may be caused by two things:

1. IE doesn't use the <embed> tag at all, it is using the <param> objects 
inside the <object> tag. However, it didn't have any effect when I tried to 
swap the scale/salign <param> tags.

2. IE is using a different Flash plugin, there were always two versions of the 
Flash plugin - one for IE (now for IE/AOL) and one for Netscape/other 
browsers. This might be the reason? I don't know but perhaps Mozilla team 
could consult Macromedia support with this issue? 


> Just chaning to sort in reversed order doesn't seem like a good fix since 
> it'll
> "break" as many testcases as it'll fix. It just happens that the one 
> attached in
> this bug belongs to the group that gets fixed.

I don't think that reversing the order would fix the error, but preserving the 
order in which the attributes appear in the file, partially because I think 
that this is the way it was done in versions 1.4 and earlier, and this seems 
to be the way it is done in Opera?

I don't know, it's just my $0.02.

Regards,
Rado

Comment 19

[Peter Lubczynski]

Comment on attachment 142011 [details] [diff] [review]
Patch for issue #2

seems okay to have this hack in our code but I'd really like a bigger comment
in the code as to why the order of attributes matters for HTML vs. XML.

Since the browser simply hands the plugin two arrays of attribute/value pairs
in NPP_New, it's really up to the plugin to interpret these. Order shouldn't
matter but we have no control over how their loop is coded. I'll let the
Macromedia engineers know about this bug and maybe future versions won't be
suspect to the same problems.

Comment 20

[Radoslav Bielik]

(In reply to comment #17)
Boris,

Thank you for clearly summarizing what I was trying to say in a complicated 
way :) Also thanks for all your help, I'll keep watching this issue and I'll 
be glad to answer any further questions you might have.

Thanks again and keep up the good work!

Regards,
Rado

Comment 21

[Johnny Stenback (:jst)]

Comment on attachment 142011 [details] [diff] [review]
Patch for issue #2

sr=jst, but yeah, a big flashing comment would be good here.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Taking.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Checked into the 1.7b trunk, with the following comment added:

  // Some plugins (eg Flash, see bug 234675.) are actually sensitive to the
  // attribute order.  So we want to make sure we give the plugin the
  // attributes in the order they came in in the source, to be compatible with
  // other browsers.  Now in HTML, the storage order is the reverse of the
  // source order, while in XML and XHTML it's the same as the source order
  // (see the AddAttributes functions in the HTML and XML content sinks).

Comment 24

[Peter Grandmaison]

I work on the Flash Plugin. Sorry for not enterring the discussion earlier, but 
this was only brought to my attention today.

An implementation detail of our player is scale and salign are stored in a 
single "WORD mode", with scale in the lower byte and salign in the upper.

The control respectively processes params scale and salign with "mode = (mode & 
~0x0f) | scaleFlags" and "mode = (mode & 0xf) | alignFlags".

The plugin handle the same attributes of the embed tag with "mode = scaleFlags" 
and "mode = mode | alignFlags".

This is unfortunate, as the plugin will ignore an salign if it's followed by a 
scale, but the control is order independant. Of course, if you consider the 
case 
  <object>
    <param name="salign" value="L" /> 
    <param name="salign" value="T" />
    <embed salign="L" salign="T" />
  </object>

then the control is undefined (either "L" or "T", but not both), but the plugin 
isn't ("LT").

It's surprising this hasn't been reported as a bug (to macromedia) -- our tools 
(Dreamweaver and Flash) emit one scale param followed by one salign param, so I 
suppose most folks start with that, and never re-order the attributes.

Which brings me to my points:

1. Given that we haven't seen this as a bug before, I feel certain all plugin 
browsers supply the attributes in the same order they were specified. If 
Mozilla broke with this tradition, alot of existing flash content would break 
with the existing players, so I think it's good that Mozilla continue to 
support source order for all document types that can embed a plugin.

2. While I loath to admit it, I don't think the flash player should fix this 
incompatibility. Consider <embed salign="r" scale="NoScale>. The current player 
ignores the salign when it's followed by scale. So it is possible that my 
existing page is meant to be left aligned, but contains the ignored salign. If 
this is fixed, then this existing page will break, as the content will right 
align. 

3. If you consider the case of repeated attributes -- again, not something 
you'd intentionally do, but it happens quite regularly -- I'm fairly sure all 
plugins are order dependant. Which re-enforces the fact that all document types 
supporting object or embed in mozilla should make sure to preserve historical 
order (despite order dependance being improper).

Comment 25

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Having the same attribute set multiple times is something that the plugin will
never see, at least in mozilla. The reason is that one of the attributes will be
dropped during parsing, an element can never ever have two attributes with the
same name. I would expect other browsers to do the same thing actually, but that
is up to them.

I was originally going to propose that since the flashplugin has this behaviour
we should make sure to always report the "scale" attribute before "salign". That
way we wouldn't be bitten by this again if we change the order attributes are
stored.

Unforunatly this means that pages such as those mentioned in point 2 in the
previous comment will "break".

It's hard to say what is more important here: logical behaviour or
backwardscompatible behaviour.
Personally i'd say that logical behaviour is better, especially if macromedias
tools generate pages that won't be affected. But if others think different then
i'm more then willing to adjust.