790915 - Motorola's website http://razr.com/ is broken after unprefixing transforms in Firefox 16

Status: RESOLVED WORKSFORME

(Tech Evangelism Graveyard :: English US, defect)

Description

[Loic]

STR:

1) Open http://razr.com/
2) Click on one of the three smartphone images

Result:
The page element doesn't slide to display the smartphone specs, only the menu tabs appear on the homepage.

Alice0775's mozregression range:

Good:
http://hg.mozilla.org/mozilla-central/rev/3be950fe9e1e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120701082951
Bad:
http://hg.mozilla.org/mozilla-central/rev/a31fc9052840
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120702030551
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3be950fe9e1e&tochange=a31fc9052840

Triggered by:
4004407a1f49 Ms2ger — Bug 745523 - Part b: Unprefix transforms; r=dbaron This patch introduces unprefixed 'transform', 'transform-origin', 'perspective', 'perspective-origin', 'transform-style' and 'backface-visibility' properties, and updates the mochitests in layout/style to use them.

It's maybe a tech evangelism bug anyway.

Reporter: http://forums.mozillazine.org/viewtopic.php?p=12283205#p12283205

Comment 1

[Boris Zbarsky [:bzbarsky]]

Do we have any idea why the unprefixing breaks this site?  Feels like it's worth understanding that...

Comment 2

[Boris Zbarsky [:bzbarsky]]

One thing I do see is that the site uses a version of the TweenMax library that only does prefixed versions of transforms.  But we left the prefixed version in place, I thought....

We should probably report that issue to TweenMax anyway, because once we drop the prefixed version their stuff _will_ break.  I can't find a way to report bugs to them, though.

Comment 3

[Loic]

(In reply to Boris Zbarsky (:bz) from comment #2)
> We should probably report that issue to TweenMax anyway, because once we
> drop the prefixed version their stuff _will_ break.  I can't find a way to
> report bugs to them, though.

Here: http://forums.greensock.com/
Or info[a]greensock.com

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Boris can you take a shot at contacting TweenMax to explain this issue to them?  I don't see that we'll need to track this but it would be good to give them as much advance notice as possible.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Lukas, Contacting TweenMax is likely not going to be enough, since even if they fix upstream the Motorola site will need to update to the fixed upstream version.

And also, it's not clear to me that the problem I mentioned in comment 2 is what's breaking the Motorola site.  See comment 2 paragraph 1 last sentence.

So what we really need here is someone to contact Motorola and/or someone to reduce the testcase to a manageable size if possible.  Something that still uses TweenMax is ok, since I can then run it against a non-minified TweenMax version.

Separately, I can contact the TweenMax folks.

David, see comment 1 and comment 2?

Comment 6

[Boris Zbarsky [:bzbarsky]]

I believe we need to track this, because we have no concrete evidence that this is not in fact our bug.  Renominating, just in case that wasn't clear before.

I'm about to collapse (it's midnight here), and I'm not going to be back until Tuesday, for what it's worth....

Comment 7

[bhavana bajaj [:bajaj]]

Joe, can you please help us find a contact at Motorola based on :bz's comment 5 ?

Comment 8

[[e-mail]]

I'll update when I find a contact !

Comment 9

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

Is there any evidence that TweenMax is even the problem here?

It looks to me like the problem is in: http://razr.com/assets/js/main.js

In particular, if you open the Web console in a mozilla-release build and in trunk, I see different results for U.Cc:
  trunk: ({vendor: true, kg: false})
  release: ({Vendor:"Moz", kg:true})
and for U.Qi() (which is called by U.fg() which is what creates U.Cc):
  trunk: 1
  release: "Moz"

There's then a bunch of code also in the U object in main.js that only works if vendor is a useful string value (e.g., it requires U.Cc.vendor + "Transform" (with a capital T!) to be a CSS property.

It's not clear to me what's going wrong in U.Qi().  The code is (with useful formatting inserted, unless I inserted the formatting wrong):

  this.Qi=function(){
    var a="transform",
        c=document.createElement("div"),
        d="Khtml ms O Moz Webkit".split(" "),
        f=d.length;
    if(a in c.style)
      return i;
    for(a=a.replace(/^[a-z]/,function(a){return a.toUpperCase()});f--;)
      if(d[f]+a in c.style)
        return d[f];
    return l
  };

l is the minified false.

Comment 10

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

Er, now I see exactly what the problem is:

(In reply to David Baron [:dbaron] from comment #9)
>     if(a in c.style)
>       return i;

Comment 11

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

If I open the site in trunk, and then, in Tools->Web Developer->Web Console, enter these 2 lines:

U.Qi = function() { return "Moz"; }
U.Cc=U.fg()

then I think the site functions correctly, or at least substantially better.

If someone could confirm that this is how the site is supposed to behave, then that confirms that it's a site bug in the Qi function.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Is there any evidence that TweenMax is even the problem here?

No, see comment 5.

Comment 13

[Boris Zbarsky [:bzbarsky]]

> If someone could confirm that this is how the site is supposed to behave

Looks identical to Firefox 15 to me.

So just to be clear, the issue is that Qi() returns "Moz" in release but ends up returning true in 16 and up, because of the code quoted in comment 10.  Then they have code like this:

            var b = a.Qi(),
                c = b + "Perspective" in document.documentElement.style || l;
            return b ? {
                vendor: b,
                kg: c
            } : l

The "kg" bit is just about whether 3d transforms are supported.  They seem to try to fall back to 2d transforms if 3d is not supported.  But in any case, things like this.Ga assume that both 2d and 3d transforms are prefixed (and in the case above would try to work with "trueTransform", I think).

So this definitely seems to be a site bug, and unrelated to TweenMax.  The question is how, or whether, we work around it....

Comment 14

[Alex Keybl [:akeybl]]

(In reply to Boris Zbarsky (:bz) from comment #13)
> So this definitely seems to be a site bug, and unrelated to TweenMax.  The
> question is how, or whether, we work around it....

As for the whether, it really depends on prevalence, and then risk to fix if prevalent.

Any ideas on prevalence of this issue on the web?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Unclear.  The relevant file is not part of any library at first glance; seems to be specific to this site.  

The question is whether we're OK breaking this site, first of all.

The only way I see to fix it is to drop the unprefixed props from CSSDeclaration for now...  Or something.

Doing a site-specific hack here is really tempting, but we just don't have the infrastructure for it right now.  :(

Comment 16

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

I think the thing to do here is just ship, which will make the site notice the problem and fix it.  We're going to hit this problem whenever we unprefix transforms, and that's something that's pretty clearly important for the Web.

Comment 17

[Boris Zbarsky [:bzbarsky]]

The question is whether they plan to fix but just won't have time in the next 10 days or whatever.

If they're just being unresponsive, I'd be tempted to agree on "just ship".

Joe, have you gotten in touch with them, by any chance?

Comment 18

[Alex Keybl [:akeybl]]

(In reply to Boris Zbarsky (:bz) from comment #17)
> The question is whether they plan to fix but just won't have time in the
> next 10 days or whatever.
> 
> If they're just being unresponsive, I'd be tempted to agree on "just ship".

I actually think we'll ship regardless, since a marketing site like razr.com functioning properly in Firefox isn't a very high priority for our users. I'm really most concerned about incompatibilities affecting Facebook/Google (or any of the other giants) or an incompatibility seen across a lot of sites on the web.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Make sense.  I doubt that this particular script will be shared by other sites, though they could of course be making similar mistakes....

Comment 20

[Karl Dubost💡 :karlcow]

tested in Firefox 32.0.3 without issues.