Closed Bug 774169 Opened 12 years ago Closed 12 years ago

Strange zooming behavior has appeared in Bing Maps

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18
Tracking Flags:
Tracking Status
firefox15 --- unaffected
firefox16 + fixed
firefox17 + fixed
firefox18 --- fixed

People

(Reporter: alice0775, Assigned: dbaron)

References

(Depends on 1 open bug,
URL
)

Details

(Keywords: regression, Whiteboard: [qa-])

Attachments

(6 files)

screen capture showing broken zoom behavior on latest nightly
308.47 KB, application/octet-stream
Details
screen capture showing proper zoom behavior on chromium
371.32 KB, application/octet-stream
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
bzbarsky
: review+
Details | Diff | Splinter Review
, patch 2: Add property_database.js entries for property aliases.
31.45 KB, 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
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
bzbarsky
: review+
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta+
Details | Diff | Splinter Review 
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
Blocks: 773929
Sorry, in comment #0, please correct as follows:
s/This is spun off from Bug 773929/This is spun off from Bug 773844/
Blocks: 773844
No longer blocks: 773929
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.
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?
Aryeh - would you be in a position to debug this? Thanks!
Assignee: nobody → ayg
status-firefox15: --- → unaffected
status-firefox16: --- → affected
status-firefox17: --- → affected
tracking-firefox16: ? → +
tracking-firefox17: --- → +
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.
Zooming issue also visible on 2012 Olympics' interactive map: http://www.london2012.com/join-in/interactive-map/
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.
Assignee: ayg → nobody
(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.
Keywords: qawanted, regressionwindow-wanted
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.
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?
Keywords: regressionwindow-wanted
Assignee: nobody → english-us
Component: Style System (CSS) → English US
Product: Core → Tech Evangelism
Version: 16 Branch → unspecified
I've sent an email to Joanne Nagel and Susan Chen to look into outreach to Microsoft's Bing Team.
For the sake of public record, what is the JS or CSS issue that you will evangelize with the bing team?
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.
I sent some mail too.
No longer blocks: 773844
If after Beta 2 of FF16 we haven't made progress with MS, we'll consider backing out bug 719054 instead for Beta 3.
Is qawanted still necessary given comment 11 and 12?
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?
Keywords: qawanted
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.
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?
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.
Assignee: english-us → JStagner
I reached out to an old MSFT contact - waiting for response.
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.
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.
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]}
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 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
Attachment #661975 - Flags: review?(bzbarsky) → review+
Comment on attachment 661976 [details] [diff] [review]
, patch 2:  Add property_database.js entries for property aliases.

r=me
Attachment #661976 - Flags: review?(bzbarsky) → review+
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
Attachment #661977 - Flags: review?(bzbarsky) → review+
Comment on attachment 661978 [details] [diff] [review]
, patch 4:  Revert bug 719054 for prefixed -moz-transform but leave it for unprefixed transform.

r=me.  :(
Attachment #661978 - Flags: review?(bzbarsky) → review+
Flags: in-testsuite+
(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 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
Attachment #661977 - Flags: approval-mozilla-aurora?
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.)
Attachment #661978 - Flags: approval-mozilla-aurora?
Attachment #661977 - Flags: approval-mozilla-beta?
Attachment #661978 - Flags: approval-mozilla-beta?
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.
Attachment #661977 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #661978 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Microsoft advises me that they now have a fix in staging
(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?
They said a week or so. Didn't sound like they had a fixed push schedule.
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.
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 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.
Attachment #661977 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #661978 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
status-firefox16: affected → fixed
(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.
Whiteboard: [qa-]
Depends on: 1098275
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: