google-code-export / flowplayer-core

Automatically exported from code.google.com/p/flowplayer-core
2 stars 0 forks source link

content plugin: behviour of tag attribute quotes reversed between 3.2.7 and 3.2.8 #510

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In 3.2.7 you HAD to do:

plugins: {
  content: {
    url: "flowplayer.content-3.2.0.swf",
    html: '<a href="http://flowplayer.org">Flowplayer</a>'
    // html: "<a href='http://flowplayer.org'>Flowplayer</a>" // does not work
  }
}

This behaviour has been reversed in 3.2.8:

plugins: {
  content: {
    url: "flowplayer.content-3.2.8.swf",
    // html: '<a href="http://flowplayer.org">Flowplayer</a>' // does not work
    html: "<a href='http://flowplayer.org'>Flowplayer</a>"
  }
}

This obviously breaks existing setups.

Original issue reported on code.google.com by blacktrashproduct on 7 Apr 2012 at 12:43

GoogleCodeExporter commented 9 years ago
This also makes our demos extremely confusing and hard to debug as the 
conventional double quoting of tag attributes works (and has to work) when 
loading the html from outside the config:
http://flowplayer.org/demos/plugins/flash/index.html#html

The claim "This could have also been written directly as a value to the html 
property." becomes more dubious.

This sample configuration would actually break:
http://flowplayer.org/plugins/flash/content.html#configuration
nevermind that it is missing a "+" for line continuation.

Original comment by blacktrashproduct on 7 Apr 2012 at 8:57

GoogleCodeExporter commented 9 years ago
The character replacements have been taken out. do you have a trunk version 
using are using ? the only difference is a change in the flex sdk used but I 
doubt it would change things that much. 

Original comment by dani...@electroteque.org on 9 Apr 2012 at 4:04

GoogleCodeExporter commented 9 years ago
No, that's the 3.2.8 and 3.2.9 release versions. And I am not alone: 
http://flowplayer.org/forum/8/98962#post-98993

Original comment by blacktrashproduct on 10 Apr 2012 at 3:02

GoogleCodeExporter commented 9 years ago
cannot replicate using both methods work however the escape method would be the 
standard way really.

//html: '<a href="http://flowplayer.org">Flowplayer</a>',
html: "<a href=\"http://flowplayer.org\">Flowplayer</a>",

Original comment by dani...@electroteque.org on 10 Apr 2012 at 4:37

GoogleCodeExporter commented 9 years ago
http://flowplayer.blacktrash.org/test/issue510.html

Original comment by blacktrashproduct on 10 Apr 2012 at 8:28

GoogleCodeExporter commented 9 years ago
html: '<a href="http://flowplayer.org">Flowplayer</a>' has been documented and 
used for years, believe me. I am NOT inventing this for God's sake.

Original comment by blacktrashproduct on 10 Apr 2012 at 8:30

GoogleCodeExporter commented 9 years ago
as i said both ways are working, cannot replicate. your example page shows a 
404. 

Original comment by dani...@electroteque.org on 10 Apr 2012 at 8:43

GoogleCodeExporter commented 9 years ago
sorry, scp'd to wrong location. 
http://flowplayer.blacktrash.org/test/issue510.html is now up.

Original comment by blacktrashproduct on 10 Apr 2012 at 9:36

GoogleCodeExporter commented 9 years ago
It's some change in flowplayer.js not in swf. Investigating.

Original comment by blacktrashproduct on 10 Apr 2012 at 9:46

GoogleCodeExporter commented 9 years ago
Ah, it's the infamous
obj = 
obj.replace(/(%)/g,"%25").replace(/'/g,'\\u0027').replace(/"/g,'\\u0022').replac
e(/&/g,'%26')
in flashembed again. Present in flowplayer-3.2.8.min.js, but removed in svn 
r775.

So, it's time for flowplayer-3.2.9.js

Original comment by blacktrashproduct on 10 Apr 2012 at 9:51

GoogleCodeExporter commented 9 years ago
Looking at that fix: shouldn't the change trigger a compilation of 
flowplayer-3.2.9.min.js instead of 3.2.8? Otherwise we have different versions 
of 3.2.8 in the will cause even more confusion.

Original comment by blacktrashproduct on 10 Apr 2012 at 9:59

GoogleCodeExporter commented 9 years ago
If you use http://releases.flowplayer.org/js/flowplayer-3.2.8.min.js you will 
still see the issue; so let's make it a proper flowplayer-3.2.9.js

Original comment by blacktrashproduct on 10 Apr 2012 at 10:02

GoogleCodeExporter commented 9 years ago
Did you want me to update you with a new js ? Yeah thats what I thought it was 
which is what I meant about the character replacements. 

Original comment by dani...@electroteque.org on 10 Apr 2012 at 11:12

GoogleCodeExporter commented 9 years ago
Not me, I can grab the stuff from svn. Our *customers/users* need 
flowplayer-3.2.9.js. Pointing *them* to 
http://code.google.com/p/flowplayer-core/source/browse/flowplayer/trunk/src/java
script/flowplayer.js/flowplayer-3.2.8.min.js?spec=svn775&r=775 will cause 
confusion, because that is _not_ flowplayer-3.2.8.min.js but 
flowplayer-3.2.8-dev.min.js or whatever we want to call it.

Original comment by blacktrashproduct on 10 Apr 2012 at 11:37

GoogleCodeExporter commented 9 years ago
I have no control of the releases obviouslly but I can compile a version and 
add it to svn if you like. 

Original comment by dani...@electroteque.org on 10 Apr 2012 at 12:22

GoogleCodeExporter commented 9 years ago
Ah, no, as far as I can see you did:
http://code.google.com/p/flowplayer-core/source/browse/flowplayer/trunk/src/java
script/flowplayer.js/flowplayer-3.2.8.min.js?spec=svn775&r=775
It's meant for whoever is in charge of releases ;-)

Original comment by blacktrashproduct on 10 Apr 2012 at 2:09

GoogleCodeExporter commented 9 years ago
yes I know we can make it a 3.2.9 build, ill do that for now ;)

Original comment by dani...@electroteque.org on 10 Apr 2012 at 2:23

GoogleCodeExporter commented 9 years ago
shall we close this now ?

Original comment by dani...@electroteque.org on 11 Apr 2012 at 12:18

GoogleCodeExporter commented 9 years ago
Personally I would prefer to close it when it is actually released. Our 
releases are not exactly a model in reliability, see: 
https://github.com/flowplayer/site/issues/250

But as long as https://github.com/flowplayer/site/issues/371 stays open, I 
guess it's ok to close.

Original comment by blacktrashproduct on 12 Apr 2012 at 8:52