Last Comment Bug 774169 - Strange zooming behavior has appeared in Bing Maps
: Strange zooming behavior has appeared in Bing Maps
Status: RESOLVED FIXED
[qa-]
: regression
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: mozilla18
Assigned To: David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
:
Mentors:
http://www.bing.com/maps/
Depends on: 1098275
Blocks: 719054
  Show dependency treegraph
 
Reported: 2012-07-15 20:48 PDT by Alice0775 White
Modified: 2014-11-13 04:33 PST (History)
11 users (show)
dbaron: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
fixed
+
fixed
fixed


Attachments
screen capture showing broken zoom behavior on latest nightly (308.47 KB, application/octet-stream)
2012-07-19 19:03 PDT, Mathieu Pellerin
no flags Details
screen capture showing proper zoom behavior on chromium (371.32 KB, application/octet-stream)
2012-07-19 19:04 PDT, Mathieu Pellerin
no flags Details
, patch 1: Make the property_database.js-based tests call getComputedStyle() for all properties that are expected to have longhand behavior. (1.64 KB, patch)
2012-09-17 17:09 PDT, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
, patch 2: Add property_database.js entries for property aliases. (31.45 KB, patch)
2012-09-17 17:09 PDT, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
, patch 3: Treat -moz-transform as a shorthand rather than an alias so the parsing function can know whether it is parsing a prefixed transform. (11.63 KB, patch)
2012-09-17 17:10 PDT, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
bzbarsky: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
, patch 4: Revert bug 719054 for prefixed -moz-transform but leave it for unprefixed transform. (11.60 KB, patch)
2012-09-17 17:10 PDT, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
bzbarsky: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Alice0775 White 2012-07-15 20:48:01 PDT
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/57abb5f70e01
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120715030544

This is spun off from Bug 773929

Steps to Reproduce:
1. Create clean profile
2. Open http://www.bing.com/maps/
3. Zoom maps by mouse wheel or +/- icon


Actual Results:
 Zooming is not smooth

Expected Results:
 Zooming should be smooth

Regression window:
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/576bbe43bb64
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120711003541
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2e3c67b2d95d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120711005141
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=576bbe43bb64&tochange=2e3c67b2d95d

Suspected:Bug 719054
Comment 1 Alice0775 White 2012-07-15 23:32:37 PDT
Sorry, in comment #0, please correct as follows:
s/This is spun off from Bug 773929/This is spun off from Bug 773844/
Comment 2 Mathieu Pellerin 2012-07-17 19:18:18 PDT
Using Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0 built from http://hg.mozilla.org/mozilla-central/rev/ae22909cef5a

Bing maps zooming still broken.
Comment 3 :Aryeh Gregor (away until August 15) 2012-07-19 08:25:10 PDT
Firebug reports no use of -moz-transform on that page, and I'm not sure how it could cause non-smooth zooming anyway.  Are you really sure that's the problem?
Comment 4 Mathieu Pellerin 2012-07-19 19:03:39 PDT
Created attachment 644129 [details]
screen capture showing broken zoom behavior on latest nightly
Comment 5 Mathieu Pellerin 2012-07-19 19:04:17 PDT
Created attachment 644130 [details]
screen capture showing proper zoom behavior on chromium
Comment 6 Alex Keybl [:akeybl] 2012-07-24 16:48:16 PDT
Aryeh - would you be in a position to debug this? Thanks!
Comment 7 Mathieu Pellerin 2012-07-24 18:25:54 PDT
Just noticed that this zooming issue is also visible on facebook as it is using bing to render dynamic maps.  This makes this bug much more visible to users.
Comment 8 Mathieu Pellerin 2012-07-27 21:35:17 PDT
Zooming issue also visible on 2012 Olympics' interactive map: http://www.london2012.com/join-in/interactive-map/
Comment 9 :Aryeh Gregor (away until August 15) 2012-07-30 04:15:57 PDT
If this needs attention soon, I don't think I can handle it, since I have too many other things on my plate right now.  Also, I don't see what reason there is to believe it's related to bug 719054.  If it is, then this is an evangelism bug.
Comment 10 Alex Keybl [:akeybl] 2012-07-30 08:29:58 PDT
(In reply to :Aryeh Gregor from comment #9)
> If this needs attention soon, I don't think I can handle it, since I have
> too many other things on my plate right now.  Also, I don't see what reason
> there is to believe it's related to bug 719054.  If it is, then this is an
> evangelism bug.

We'll get a more granular regression window in that case. Adding qawanted to look at hourlies (if possible). If for whatever reason those aren't available, we can send this bug over to dbaron and see if he's be willing to throw together a try build with and without bug 719054 for testing.
Comment 11 XtC4UaLL [:xtc4uall] 2012-07-30 15:54:52 PDT
FWIW, I get the same Regression Window using Hourly Builds (located in http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/?C=M;O=D) as Alice mentioned in Comment 0.
Comment 12 XtC4UaLL [:xtc4uall] 2012-07-30 16:51:58 PDT
In Addition, you get 

Error in parsing value for '-moz-transform'.  Declaration dropped. @ http://www.bing.com/maps/

Output in Web/Error Console.
See also Bug 774475 for a similar Issue on another Site. This is certainly a TE Bug.
Anyone able to contact Microsoft's Bing Team about this?
Comment 13 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-08 09:59:51 PDT
I've sent an email to Joanne Nagel and Susan Chen to look into outreach to Microsoft's Bing Team.
Comment 14 Mathieu Pellerin 2012-08-08 19:32:59 PDT
For the sake of public record, what is the JS or CSS issue that you will evangelize with the bing team?
Comment 15 Boris Zbarsky [:bz] 2012-08-08 21:49:16 PDT
The issue is that they're using matrix() or matrix3d() with length units in the translate components in their -moz-transforms.  But presumably not in their -webkit-transforms, since those transform values never supported length units in WebKit.
Comment 16 Boris Zbarsky [:bz] 2012-08-08 21:57:01 PDT
I sent some mail too.
Comment 17 Alex Keybl [:akeybl] 2012-08-22 17:35:18 PDT
If after Beta 2 of FF16 we haven't made progress with MS, we'll consider backing out bug 719054 instead for Beta 3.
Comment 18 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-08-28 13:42:59 PDT
Is qawanted still necessary given comment 11 and 12?
Comment 19 Alex Keybl [:akeybl] 2012-08-29 10:31:54 PDT
Nope, Alice's original range sounds good. Removing qawanted.

(In reply to Boris Zbarsky (:bz) from comment #16)
> I sent some mail too.

Have you received a response?
Comment 20 Alex Keybl [:akeybl] 2012-09-05 16:40:55 PDT
We've decided that this bug's current state isn't critical enough to force a backout of bug 719054. We'll continue to track for release and follow up with Stormy's team about outreach.
Comment 21 Mathieu Pellerin 2012-09-05 18:41:33 PDT
Hmm; this bug's current state is affecting facebook dynamic maps behavior. While I'm not sure how popular bing map is, we're aware of facebook users metric :)

The bug title is misleading in that it doesn't mention the more visited affected websites. Did the decision not to backout bug 719054 take into account fact that firefox users will encounter a broken behavior while on facebook?
Comment 22 Alex Keybl [:akeybl] 2012-09-06 07:46:54 PDT
Mathieu - Joe's going to try to do further outreach to the Bing Maps team on Monday. If MS leaves this unfixed, it wouldn't be a critical user regression. Maps are still totally usable.

dbaron should be able to speak to the reason for fixing bug 719054 in the same release that we unprefix. If he doesn't feel strongly about leaving it in, we can reconsider backing bug 719054 out.
Comment 23 JStagner@Mozilla.com 2012-09-10 07:49:56 PDT
I reached out to an old MSFT contact - waiting for response.
Comment 24 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-12 11:35:55 PDT
I wouldn't consider a backout as an option here.

If we need to take a temporary fix for this, it would be to condition the change whether we accept length units on whether the property is prefixed (i.e., revert the change for -moz-transform and keep it for transform).  That's a somewhat more involved patch and I wouldn't want to land it at the very last minute.
Comment 25 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-12 23:15:22 PDT
Oh, and in case it wasn't clear:  what Bing needs to do to fix this is also include use of unprefixed 'transform', with the same syntax they use for -webkit-transform.
Comment 26 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-12 23:25:46 PDT
And the JS stacks at which the Bing maps code is setting the bad -moz-transform all look like this, I think:

0 anonymous() ["http://ecn.dev.virtualearth.net/mapcontrol/v7.0/js/bin/7.0.201208
30112127.00/en-us/veapicore.js":1]
    this = [object Object]
1 tt() ["http://ecn.dev.virtualearth.net/mapcontrol/v7.0/js/bin/7.0.2012083011212
7.00/en-us/veapicore.js":1]
    this = [object Window @ 0x3d6adbf0 (native @ 0x1208e4a0)]
2 anonymous() ["http://ecn.dev.virtualearth.net/mapcontrol/v7.0/js/bin/7.0.201208
30112127.00/en-us/veapicore.js":1]
    this = [object Object]
3 si() ["http://ecn.dev.virtualearth.net/mapcontrol/v7.0/js/bin/7.0.2012083011212
7.00/en-us/veapicore.js":1]
    this = [object Window @ 0x3d6adbf0 (native @ 0x1208e4a0)]
4 o() ["http://ecn.dev.virtualearth.net/mapcontrol/v7.0/js/bin/7.0.20120830112127
.00/en-us/veapicore.js":1]
    this = [object Window @ 0x3d6adbf0 (native @ 0x1208e4a0)]
5 anonymous() ["http://ecn.dev.virtualearth.net/mapcontrol/v7.0/js/bin/7.0.201208
30112127.00/en-us/veapicore.js":1]
    this = function (){i[t]&&n(+new Date),delete i[t]}
Comment 27 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-17 16:18:23 PDT
Alex asked me earlier today:

[2012-09-17 11:48:26] <akeybl> just wanted to hear if you thought a temporary fix would be low risk, and if so, whether we could get it into the build tomorrow (beta 4)

I think it would be reasonably low-risk, but I don't think getting it into the build tomorrow is realistic.  I've been working on it this afternoon and I have most of it written at this point.
Comment 28 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-17 17:09:40 PDT
Created attachment 661975 [details] [diff] [review]
, patch 1:  Make the property_database.js-based tests call getComputedStyle() for all properties that are expected to have longhand behavior.
Comment 29 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-17 17:09:53 PDT
Created attachment 661976 [details] [diff] [review]
, patch 2:  Add property_database.js entries for property aliases.
Comment 30 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-17 17:10:05 PDT
Created attachment 661977 [details] [diff] [review]
, patch 3:  Treat -moz-transform as a shorthand rather than an alias so the parsing function can know whether it is parsing a prefixed transform.
Comment 31 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-17 17:10:19 PDT
Created attachment 661978 [details] [diff] [review]
, patch 4:  Revert bug 719054 for prefixed -moz-transform but leave it for unprefixed transform.
Comment 32 Boris Zbarsky [:bz] 2012-09-18 11:08:21 PDT
Comment on attachment 661975 [details] [diff] [review]
, patch 1:  Make the property_database.js-based tests call getComputedStyle() for all properties that are expected to have longhand behavior.

r=me
Comment 33 Boris Zbarsky [:bz] 2012-09-18 11:10:09 PDT
Comment on attachment 661976 [details] [diff] [review]
, patch 2:  Add property_database.js entries for property aliases.

r=me
Comment 34 Boris Zbarsky [:bz] 2012-09-18 11:11:40 PDT
Comment on attachment 661977 [details] [diff] [review]
, patch 3:  Treat -moz-transform as a shorthand rather than an alias so the parsing function can know whether it is parsing a prefixed transform.

r=me
Comment 35 Boris Zbarsky [:bz] 2012-09-18 11:13:05 PDT
Comment on attachment 661978 [details] [diff] [review]
, patch 4:  Revert bug 719054 for prefixed -moz-transform but leave it for unprefixed transform.

r=me.  :(
Comment 37 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-18 12:56:02 PDT
also https://hg.mozilla.org/integration/mozilla-inbound/rev/5321d02d564b
Comment 38 Alex Keybl [:akeybl] 2012-09-18 13:10:22 PDT
(In reply to David Baron [:dbaron] from comment #27)
> Alex asked me earlier today:
> 
> [2012-09-17 11:48:26] <akeybl> just wanted to hear if you thought a
> temporary fix would be low risk, and if so, whether we could get it into the
> build tomorrow (beta 4)
> 
> I think it would be reasonably low-risk, but I don't think getting it into
> the build tomorrow is realistic.  I've been working on it this afternoon and
> I have most of it written at this point.

Thanks dbaron, let's get this onto Aurora whenever ready and then we'll uplift for Beta 5.
Comment 39 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-18 13:54:23 PDT
Comment on attachment 661977 [details] [diff] [review]
, patch 3:  Treat -moz-transform as a shorthand rather than an alias so the parsing function can know whether it is parsing a prefixed transform.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 719054
User impact if declined: the zoom animation on bing maps (and other sites that use them) shows a different region of the map instead of an animation
Testing completed (on m-c, etc.): on mozilla-inbound currently
Risk to taking this patch (and alternatives if risky): relatively low-risk, since the intentional behavior change is a conditional backout of bug 719054 (conditioned on whether the property being set is prefixed or not), although there's the possibility that there's an observable difference between aliasing and the shorthand behavior that I missed
String or UUID changes made by this patch: none
Comment 40 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-18 13:55:00 PDT
Comment on attachment 661978 [details] [diff] [review]
, patch 4:  Revert bug 719054 for prefixed -moz-transform but leave it for unprefixed transform.

[Approval Request Comment]
see previous comment

(I'm not bothering with approval requests for patches 1 and 2 since they're test-only and therefore I'm presuming they don't need approval.)
Comment 42 Alex Keybl [:akeybl] 2012-09-19 17:18:00 PDT
Comment on attachment 661977 [details] [diff] [review]
, patch 3:  Treat -moz-transform as a shorthand rather than an alias so the parsing function can know whether it is parsing a prefixed transform.

[Triage Comment]
Approving for Aurora in preparation for Beta landing.
Comment 44 JStagner@Mozilla.com 2012-09-20 05:08:49 PDT
Microsoft advises me that they now have a fix in staging
Comment 45 Alex Keybl [:akeybl] 2012-09-20 10:25:30 PDT
(In reply to JStagner@Mozilla.com from comment #44)
> Microsoft advises me that they now have a fix in staging

Thanks for following up! Any word on when the fix will roll out?
Comment 46 JStagner@Mozilla.com 2012-09-20 11:43:16 PDT
They said a week or so. Didn't sound like they had a fixed push schedule.
Comment 47 Lukas Blakk [:lsblakk] use ?needinfo 2012-09-21 15:40:13 PDT
If this is landed and Microsoft's fix on their end is rolled out is there any negative interactions or fallout this could cause?  If that's a possibility, we should hold off on landing this and keep checking back for Microsoft's fix to roll out. Otherwise, if it's known to be safe, we can land this knowing that no matter what happens on Microsoft's end - we're good to ship 16.
Comment 48 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-21 20:11:12 PDT
I think it's unlikely to have any bad interactions; I think we should probably land this.

(There's still some risk from the patch in general, mainly from the possibility that while the change mostly tried not to change other things, it may not have succeeded.)
Comment 49 Alex Keybl [:akeybl] 2012-09-24 09:52:09 PDT
Comment on attachment 661977 [details] [diff] [review]
, patch 3:  Treat -moz-transform as a shorthand rather than an alias so the parsing function can know whether it is parsing a prefixed transform.

[Triage Comment]
Since this will land before beta 5 goes to build tomorrow, I think we can still manage this risk. Agreed that we should land on beta.
Comment 50 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-09-24 10:47:54 PDT
Landed:
   https://hg.mozilla.org/releases/mozilla-beta/rev/6580000865f2
   https://hg.mozilla.org/releases/mozilla-beta/rev/3ce450f5bd51
   https://hg.mozilla.org/releases/mozilla-beta/rev/bfa037b4d51e
   https://hg.mozilla.org/releases/mozilla-beta/rev/ff0032b0ad00

I'm not going to be able to watch the tree since I have to head to the airport in a few minutes, but that's the problem with combining a "you must watch the tree for hours" policy with "you must land within 20 hours" approvals.
Comment 51 Alex Keybl [:akeybl] 2012-09-24 13:39:55 PDT
(In reply to David Baron [:dbaron] from comment #50)
> I'm not going to be able to watch the tree since I have to head to the
> airport in a few minutes

Landing looks good so far.

Note You need to log in before you can comment on or make changes to this bug.