Closed Bug 790761 Opened 12 years ago Closed 12 years ago

http://www.apple.com/iphone/ nav links failing

Categories

(Tech Evangelism Graveyard :: English US, defect)

Product:
Tech Evangelism Graveyard
Component:
English US
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox16-, firefox17-, firefox18-)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox16 - ---
firefox17 - ---
firefox18 - ---

People

(Reporter: dherman, Assigned: dbaron)

References

(
URL
)

Details

(Keywords: regression) 

In Nightly, the little circular nav links at the bottom of the hero unit stop working after the first time you click on one. dbaron and I looked into it briefly and couldn't find a JS error or figure out what code was supposed to be registered on the event.

Dave
Seems likely (just based on the fact that it's an Apple Web site) to be the unprefixing of transitions and animations (bug 762302 and bug 762303).
That's my current hypothesis too.  Bisecting just to check.
Bisect says:
The first bad revision is:
changeset:   98674:137f4655cf25
user:        Emmanuele Bassi <ebassi@mozilla.com>
date:        Sun Jul 08 21:25:10 2012 -0400
summary:     Bug 762303 - Unprefix CSS Transition properties and provide temporary aliases for -moz-transition and exposed subproperties. r=dbaron

There's a pretty good chance this is a bug in the site...
Blocks: 762303
Version: unspecified → 16 Branch
Keywords: regressionwindow-wanted
(In reply to Boris Zbarsky (:bz) from comment #5)
> Bisect says:
> The first bad revision is:
> changeset:   98674:137f4655cf25
> user:        Emmanuele Bassi <ebassi@mozilla.com>
> date:        Sun Jul 08 21:25:10 2012 -0400
> summary:     Bug 762303 - Unprefix CSS Transition properties and provide
> temporary aliases for -moz-transition and exposed subproperties. r=dbaron
> 
> There's a pretty good chance this is a bug in the site...

That doesn't really make sense. If it works on Firefox 15, then it should work on the later. If it doesn't, it means we have a compatibility issue and people are not going to want to use firefox. Not to mention web designers will be furious. Also, just so everyone knows, the bug is also located here on their website: http://www.apple.com/ipod-touch/design/#ipod-touch-fluidgallery
> If it works on Firefox 15, then it should work on the later.

Unless the site is going out of its way to sniff Firefox versions and do stuff differently in different versions.  Which Apple has done before.

> it means we have a compatibility issue

That much is true.  The question is, compatibility with _what_?

I have no idea why this is not getting tracked...
tracking-firefox16: --- → ?
tracking-firefox17: --- → ?
tracking-firefox18: --- → ?
Alice/Martijn - can either of you take a stab at minimizing the test case? Much appreciated!
So I started doing some splitting of the patch.  Not surprisingly, the problem results not from adding support for the properties to the CSS parser and to getPropertyValue()/setProperty(), but from adding the new properties to nsIDOMCSS2Properties.  (Next step:  which properties?)
Just exposing "transition" on nsIDOMCSS2Properties breaks the site.
(In reply to David Baron [:dbaron] from comment #11)
> Just exposing "transition" on nsIDOMCSS2Properties breaks the site.

To be clear, this was in addition to with implementing support for all the properties unprefixed, but with no nsIDOMCSS2Properties changes (which didn't break the site on its own, without the one additional change).

Additionally, I tested adding the other four transition* properties to nsIDOMCSS2Properties (but not adding transition) (again, on top of adding support for the unprefixed properties without any nsIDOMCSS2Properties changes), and that did not break the site.

So the key aspect here is a test of nsIDOMCSS2Properties.transition.
OK, I found the problem in Apple's code, and it's very clearly a problem on Apple's site.

The problem is in the function setVendorPrefixStyle in http://images.apple.com/global/scripts/apple_core.js .  (The same function is present in http://images.apple.com/global/ac_base/ac_base.js , but that copy isn't used.)

The function is currently:

setVendorPrefixStyle:function(c,f,e){if(f.match(/^webkit/i)){f=f.replace(/^webkit/i,"")
}else{if(f.match(/^moz/i)){f=f.replace(/^moz/i,"")}else{if(f.match(/^ms/i)){f=f.replace(/^ms/i,"")
}else{if(f.match(/^o/i)){f=f.replace(/^o/i,"")}else{if(f.match("-")){var b=f.split("-"),d=b.length;
f="";for(var a=0;a<b.length;a++){f+=b[a].charAt(0).toUpperCase()+b[a].slice(1)}}else{f=f.charAt(0).toUpperCase()+f.slice(1)
}}}}}if(e.match("-webkit-")){e=e.replace("-webkit-","-vendor-")}else{if(e.match("-moz-")){e=e.replace("-moz-","-vendor-")
}else{if(e.match("-ms-")){e=e.replace("-ms-","-vendor-")}else{if(e.match("-o-")){e=e.replace("-o-","-vendor-")
}}}}c.style["webkit"+f]=e.replace("-vendor-","-webkit-");c.style["Moz"+f]=e.replace("-vendor-","-moz-");
c.style["ms"+f]=e.replace("-vendor-","-ms-");c.style["O"+f]=e.replace("-vendor-","-o-");
c.style[f]=e;f=f.charAt(0).toLowerCase()+f.slice(1);c.style[f]=e}

Each time I click on the little circular nav links in question, there are two calls to this function.  The first has arguments:
  f=transition
  e=-webkit-transform 0.4s cubic-bezier(0,0,0.25,1)
and the second has arguments:
  f=transform
  e=translate(-1360px, 0)

It's the first of these calls that's the problem, because what they end up leading to is, at the very end:

  c.style["transition"]="-vendor-transform 0.4s cubic-bezier(0,0,0.25,1)"
  c.style["transform"]="translate(-1360px, 0)"

In particular, once we unprefix "transition", this JS code (quoted above) tries to set the "-vendor-transform" property instead of the "-moz-transform" property.
(In reply to David Baron [:dbaron] from comment #13)
> In particular, once we unprefix "transition", this JS code (quoted above)
> tries to set the "-vendor-transform" property instead of the
> "-moz-transform" property.

Er, instead of "set the", I should have said "transition the".

(And then, presumably, since the site functions based on transitionEnd events, the lack of a functioning transition means that the change doesn't actually take effect.)
And, for further confirmation of this theory, the following 4 bash commands will produce a copy of the site that doesn't have this bug (amazingly, Apple uses absolute URLs intensively, so it's very easy to debug their site from a different server):

$ wget http://www.apple.com/iphone/ -O index.html
$ wget http://images.apple.com/global/scripts/apple_core.js
$ sed -i -e 's,http://images.apple.com/global/scripts/apple_core.js,apple_core.js,' index.html
$ sed -i -e 's/"-webkit-","-vendor-"/"-webkit-",""/' apple_core.js
The fourth line above is a hack fix.  Now that I've read through the function more carefully, the correct fix is to replace the fourth line above with:

$ sed -i -e 's/c.style\[f\]=e/c.style[f]=e.replace("-vendor-","")/g' apple_core.js




To restate what's going on in Apple's website in English:  they're setting multiple vendor-prefixed versions of the property, and for each one, they're making vendor prefix substitutions on the value. However, they're not doing the substitution when they set the unprefixed form, so that what they do is essentially:

  c.style["webkitTransition"]="-webkit-transform 0.4s cubic-bezier(0,0,0.25,1)"
  c.style["MozTransition"]="-moz-transform 0.4s cubic-bezier(0,0,0.25,1)"
  c.style["msTransition"]="-ms-transform 0.4s cubic-bezier(0,0,0.25,1)"
  c.style["OTransition"]="-o-transform 0.4s cubic-bezier(0,0,0.25,1)"
  c.style["Transition"]="-vendor-transform 0.4s cubic-bezier(0,0,0.25,1)"
  c.style["transition"]="-vendor-transform 0.4s cubic-bezier(0,0,0.25,1)"

The substitution in this comment fixes just the last two sets in the script (whereas the substitution I'd tried in the earlier comment simply gets rid of the thing they were substituting so that all the values are unprefixed, which was good enough for testing the problem but isn't actually a fix).
Oops, and I meant to add:  all 6 of those sets in comment 16 are either (a) no-ops or (b) overwrite what was done previously.  So even though the set of a MozTransition value is good, now that we support "transition" as well, the bad value overwrites the good one.
I emailed a contact at Apple about this (though not one who works on www.apple.com).  If somebody else has contacts there, it might be a good idea to try them as well, though.

I don't think there's anything reasonable to do about this from our side.  It's very clearly a site bug.
Assignee: nobody → dbaron
Component: General → English US
OS: Mac OS X → All
Product: Core → Tech Evangelism
Hardware: x86 → All
Version: 16 Branch → unspecified
I did get a response:
  # > I'm hoping you can forward this to somebody who works on
  # > www.apple.com.
  # 
  # They've been made aware of the issue and are looking into it.
(In reply to David Baron [:dbaron] from comment #18)
> I emailed a contact at Apple about this (though not one who works on
> www.apple.com).  If somebody else has contacts there, it might be a good
> idea to try them as well, though.
> 
> I don't think there's anything reasonable to do about this from our side. 
> It's very clearly a site bug.

Given this is a site bug and not a Gecko bug, untracking for upcoming releases.
tracking-firefox16: + → -
tracking-firefox17: + → -
tracking-firefox18: + → -
My contact at Apple emailed to say the changes to fix this are now live.
(In reply to David Baron [:dbaron] (recovering from illness; hopefully back Oct 8, but with backlog) from comment #13)
> The problem is in the function setVendorPrefixStyle in
> http://images.apple.com/global/scripts/apple_core.js .  (The same function
> is present in http://images.apple.com/global/ac_base/ac_base.js , but that
> copy isn't used.)

I confirmed by code inspection that both copies of the function are fixed.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Blocks: 114156
Product: Tech Evangelism → Tech Evangelism Graveyard
You need to log in before you can comment on or make changes to this bug.