Pages navigated to by following links from Facebook app on B2G cannot be zoomed

VERIFIED FIXED in Firefox 36, Firefox OS v2.1

Status

()

Core
Document Navigation
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: cwiiis, Assigned: cwiiis)

Tracking

({regression})

unspecified
2.1 S8 (7Nov)
All
Gonk (Firefox OS)
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1+, firefox34 wontfix, firefox35 wontfix, firefox36 fixed, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 verified)

Details

(Whiteboard: [systemsfe], URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
[Blocking Requested - why for this release]: Lots of desktop sites are basically unusable

I come across a lot of desktop pages on b2g that can't be zoomed in the system browser for no apparent reason (they have no meta viewport and they scroll on both axes). I've put such an example in the URL.

Such pages are basically unreadable. I believe this is a regression, but I've no idea when exactly this happened (as, thankfully, most sites are responsive/have mobile versions).

This affects 2.1 and I think is a serious enough UX regression to block on. I don't believe this affects fennec, but I'll test this later.
(Assignee)

Comment 1

3 years ago
Can confirm this doesn't affect fennec beta at least.
On a Flame running relatively recent code (a few days ago) I can zoom just fine on the URL provided. Botond, are you able to reproduce this?
Flags: needinfo?(botond)
(Assignee)

Comment 3

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> On a Flame running relatively recent code (a few days ago) I can zoom just
> fine on the URL provided. Botond, are you able to reproduce this?

This is odd, I flashed 2.1 to confirm and reproduced... Maybe it was my method of going to the link (via a Facebook link) that causes the issue...? Pasting for reference, will try to confirm as well. https://www.facebook.com/l.php?u=https%3A%2F%2Fwww.techdirt.com%2Farticles%2F20141015%2F13561128840%2Ffirst-commerce-department-fines-intel-subsidiary-exporting-encryption.shtml&h=cAQFzJjuDAQEQjQkekcFUEpVT_7YwO1_OdleQEECBc1Q3Og&enc=AZNwfT_eaehWR4st2xJJoieXoBMZl9OoNF65wFzj_D34xqZqQbO0m-uyXfbeySIUQ95kT45V_dOVjKhaM_3MdvOODYZfSkq-mAEXEp1p3UWBQvY6_IMaMgZHAV7E_eJF5qfrQN9LqOnwInAdnXgrkbSb&s=1
(Assignee)

Comment 4

3 years ago
Following either link from this bug does not reproduce the error, so perhaps it's only windows opened via Facebook that exhibit it?
(Assignee)

Comment 5

3 years ago
Confirmed.

STR:

1. Log into facebook via the Facebook app from the marketplace (not tested via website)
2. Follow a link to a site that would ordinarily be zoomable

Expected:

Page opens, can be zoomed

Actual:

Page opens, cannot be zoomed
Summary: Many pages can't be zoomed for no reason → Pages navigated to by following links from Facebook on B2G cannot be zoomed
(Assignee)

Comment 6

3 years ago
This does not occur if you visit facebook via the website rather than the app (which is exactly the same, just packaged differently).

I expect this applies to all links followed from apps that open in an external window, but I'll leave the bug to reference what I've verified.
Summary: Pages navigated to by following links from Facebook on B2G cannot be zoomed → Pages navigated to by following links from Facebook app on B2G cannot be zoomed
Flags: needinfo?(botond)
Do you know what code is invoked when apps open links? Presumably there's a window.open call somewhere in Gaia that is spawning the new window.
(Assignee)

Comment 8

3 years ago
I assume this file handles the request? https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/child_window_factory.js
(Assignee)

Comment 9

3 years ago
So the event gets sent here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/child_window_factory.js#L147

and handled here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/wrapper_factory.js#L15
which looks like it creates the AppWindow here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window_factory.js#L194

this ends up in this block: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L544

So it uses the iframe from the 'frameElement' detail on the original mozbrowseropenwindow event... Not sure where this comes from just yet.
(Assignee)

Comment 10

3 years ago
(In reply to Chris Lord [:cwiiis] from comment #9)
> So the event gets sent here:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> child_window_factory.js#L147
> 
> and handled here:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> wrapper_factory.js#L15

Ordering slightly wrong here, child_window_factory handles it, not wrapper_factory (which only handles window.open with remote=true). I guess the problem here is the resulting iframe isn't the root frame, so doesn't get zooming(?)

Unfortunate, but if that's the case, I suppose this can't really be a blocker. Is there a reason why _blank target pages don't get remote frames? Visually, they're handled exactly the same now (vs. how it was pre-2.1(/2.0?)).
(Assignee)

Comment 11

3 years ago
I'm guessing this chain of events is initiated Gecko-side by OpenWindowInProcess http://mxr.mozilla.org/mozilla-central/ident?i=OpenWindowInProcess (as opposed to OpenWindowOOP http://mxr.mozilla.org/mozilla-central/ident?i=OpenWindowOOP)
(Assignee)

Comment 12

3 years ago
I guess this is also the reason why navigating to YouTube links from Facebook doesn't let you fullscreen videos(?)
Component: Panning and Zooming → Gaia::System::Window Mgmt
Product: Core → Firefox OS
Version: Trunk → unspecified
Adding qawanted to reproduce this and to check 2.0 for a possible regression window.
Keywords: qawanted
QA Contact: jmercado
This issue DOES occur on Flame KK 2.2 and Flame KK 2.1 shallow flash.

Environmental Variables:
Device: Flame 2.2
BuildID: 20141020055012
Gaia: dc496d04907dd314f9736ff78bab3bd27156f79a
Gecko: f2d7d694aae5
Version: 36.0a1 (2.2) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Environmental Variables:
Device: Flame 2.1
BuildID: 20141020105810
Gaia: dc26d4a1c28682137130c62058399b43a4ea86b4
Gecko: 7e96ff7c7a64
Version: 34.0 (2.1) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

This issue does NOT occur on Flame KK 2.0 shallow flash after 10 attempts.

Results: 
Environmental Variables:
Device: Flame 2.0
BuildID: 20141020025114
Gaia: 63b56a7a7453726b9e12ad1afe02c68c83c5aeca
Gecko: 09b9387be5ad
Version: 32.0 (2.0) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted → regression
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0: --- → unaffected
Flags: needinfo?(jmitchell)
Keywords: regressionwindow-wanted

Updated

3 years ago
Whiteboard: [systemsfe]
definite broken functionality.
blocking-b2g: 2.1? → 2.1+
Assignee: nobody → chrislord.net
Target Milestone: --- → 2.1 S7 (24Oct)
(Assignee)

Comment 16

3 years ago
Before I dig further into this, do you have any comment on this Alive? (knowing the window management code a lot better than I do)
Flags: needinfo?(alive)
Bug 1060191 seems to be the cause of this issue.

B2g-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 2.2
BuildID: 20140926001739
Gaia: 40dff263f02982b47fe548c403cd5b57d69b8d18
Gecko: 41ae5078b2ff
Version: 35.0a1 (2.2) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Broken 
Environmental Variables:
Device: Flame 2.2
BuildID: 20140926003241
Gaia: 6163c5ee0f99f73439318d064e0862ccd4e11a49
Gecko: ce0b48c0f6d1
Version: 35.0a1 (2.2) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Last Working gaia / First Broken gecko - Issue does NOT occur
Gaia: 40dff263f02982b47fe548c403cd5b57d69b8d18
Gecko: ce0b48c0f6d1

First Broken gaia / Last Working gekko - Issue DOES occur
Gaia: 6163c5ee0f99f73439318d064e0862ccd4e11a49
Gecko: 41ae5078b2ff

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/40dff263f02982b47fe548c403cd5b57d69b8d18...6163c5ee0f99f73439318d064e0862ccd4e11a49
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: regressionwindow-wanted
Broken by Bug 1060191 

Chris - since you are the 'Assigned To' on this bug I'm NI'ing you instead of the Patch Author
Blocks: 1060191
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell) → needinfo?(chrislord.net)
(In reply to Chris Lord [:cwiiis] from comment #16)
> Before I dig further into this, do you have any comment on this Alive?
> (knowing the window management code a lot better than I do)

I guess we are not doing something for APZ enabling for the mozbrowser iframe provided by mozbrowseropenwindow event detail.iframe coming from gecko?

What bug 1060191 did is, we were always creating a new mozbrowser iframe in gaia side for every mozbrowseropenwindow event in the past, but now we tend to re-use the iframe given by gecko.
Flags: needinfo?(alive)
(Assignee)

Comment 20

3 years ago
I'm not convinced that this is a Gaia bug at all - bug 1060191 changes things so that rather than Gaia, incorrectly, creating a new window for every window.open, it reuses the iframe that comes from event detail, which I presume is created by Gecko.

So this iframe that we get from platform doesn't seem to support zooming for whatever reason (my first assumption would be that it's not the root frame) and I'm not sure there's anything we can do about that? (unless we want to break spec and revert bug 1060191)

I'll look into this and confirm/obsolete these assumptions.
Flags: needinfo?(chrislord.net)
(Assignee)

Comment 21

3 years ago
Well, this is puzzling - it seems not all windows opened from Facebook aren't zoomable, only some... All the opened windows are remote, so they ought to be zoomable - so this could well be a Gaia problem I suppose, but it's weird that some are zoomable and some aren't... I've added some logging in various bits of Gaia and Gecko.

Here's launching a zoomable link:

I/Gecko   ( 5063): XXX: OpenWindowOOP
I/GeckoConsole( 5063): Content JS LOG: XXX Opening window: {"features":"","frameElement":{},"name":"_blank","url":"http://lm.facebook.com/l.php?u=http%3A%2F%2Ft.co%2FYlEvX0Buvb&h=dAQE73K7f&enc=AZOw-41Ac9wSL0JjIeJaTe3gsfOltaeaw8X6pt5E1iSWX8UyQWZWho-L5dCetTlMWNgExtX0GYr8MCuL8Hf903QYh6BsSpgHgv1kTNvYwRHUsm04hTF2W-qu9dNXlUxUfu753kdafcocP8wp88dkVLsD3b3Wxoa0pfbzjOEUKYqzrQ&s=1"} 
I/GeckoConsole( 5063): Content JS LOG: XXX Ignoring, not remote 
I/GeckoConsole( 5063): Content JS LOG: XXX Creating new window: {"url":"http://lm.facebook.com/l.php?u=http%3A%2F%2Ft.co%2FYlEvX0Buvb&h=dAQE73K7f&enc=AZOw-41Ac9wSL0JjIeJaTe3gsfOltaeaw8X6pt5E1iSWX8UyQWZWho-L5dCetTlMWNgExtX0GYr8MCuL8Hf903QYh6BsSpgHgv1kTNvYwRHUsm04hTF2W-qu9dNXlUxUfu753kdafcocP8wp88dkVLsD3b3Wxoa0pfbzjOEUKYqzrQ&s=1","name":"_blank","iframe":{}} 
I/GeckoConsole( 5063): Content JS LOG: XXX Creating AppWindow: {"url":"http://lm.facebook.com/l.php?u=http%3A%2F%2Ft.co%2FYlEvX0Buvb&h=dAQE73K7f&enc=AZOw-41Ac9wSL0JjIeJaTe3gsfOltaeaw8X6pt5E1iSWX8UyQWZWho-L5dCetTlMWNgExtX0GYr8MCuL8Hf903QYh6BsSpgHgv1kTNvYwRHUsm04hTF2W-qu9dNXlUxUfu753kdafcocP8wp88dkVLsD3b3Wxoa0pfbzjOEUKYqzrQ&s=1","iframe":{},"name":"_blank","origin":"http://lm.facebook.com/l.php?u=http%3A%2F%2Ft.co%2FYlEvX0Buvb&h=dAQE73K7f&enc=AZOw-41Ac9wSL0JjIeJaTe3gsfOltaeaw8X6pt5E1iSWX8UyQWZWho-L5dCetTlMWNgExtX0GYr8MCuL8Hf903QYh6BsSpgHgv1kTNvYwRHUsm04hTF2W-qu9dNXlUxUfu753kdafcocP8wp88dkVLsD3b3Wxoa0pfbzjOEUKYqzrQ&s=1","manifestURL":"","manifest":null,"evtType":"openwindow"} 
I/GeckoConsole( 5063): Content JS LOG: XXX We have an iframe 

Here's launching an unzoomable link:

I/Gecko   ( 5063): XXX: OpenWindowOOP
I/GeckoConsole( 5063): Content JS LOG: XXX Opening window: {"features":"","frameElement":{},"name":"_blank","url":"https://m.facebook.com/l.php?u=https%3A%2F%2Fwww.techdirt.com%2Farticles%2F20141015%2F13561128840%2Ffirst-commerce-department-fines-intel-subsidiary-exporting-encryption.shtml&h=RAQFyr2t6&enc=AZMA4lw7C8TYB9cwVGIHewnFZMFuYvyp9lWLzo0bgrwuBNJ4xK_4o5YdHeQkuG93MWYrKp2PK4SygC9ldzCtCnGf&s=1"} 
I/GeckoConsole( 5063): Content JS LOG: XXX Ignoring, not remote 
I/GeckoConsole( 5063): Content JS LOG: XXX Creating new window: {"url":"https://m.facebook.com/l.php?u=https%3A%2F%2Fwww.techdirt.com%2Farticles%2F20141015%2F13561128840%2Ffirst-commerce-department-fines-intel-subsidiary-exporting-encryption.shtml&h=RAQFyr2t6&enc=AZMA4lw7C8TYB9cwVGIHewnFZMFuYvyp9lWLzo0bgrwuBNJ4xK_4o5YdHeQkuG93MWYrKp2PK4SygC9ldzCtCnGf&s=1","name":"_blank","iframe":{}} 
I/GeckoConsole( 5063): Content JS LOG: XXX Creating AppWindow: {"url":"https://m.facebook.com/l.php?u=https%3A%2F%2Fwww.techdirt.com%2Farticles%2F20141015%2F13561128840%2Ffirst-commerce-department-fines-intel-subsidiary-exporting-encryption.shtml&h=RAQFyr2t6&enc=AZMA4lw7C8TYB9cwVGIHewnFZMFuYvyp9lWLzo0bgrwuBNJ4xK_4o5YdHeQkuG93MWYrKp2PK4SygC9ldzCtCnGf&s=1","iframe":{},"name":"_blank","origin":"https://m.facebook.com/l.php?u=https%3A%2F%2Fwww.techdirt.com%2Farticles%2F20141015%2F13561128840%2Ffirst-commerce-department-fines-intel-subsidiary-exporting-encryption.shtml&h=RAQFyr2t6&enc=AZMA4lw7C8TYB9cwVGIHewnFZMFuYvyp9lWLzo0bgrwuBNJ4xK_4o5YdHeQkuG93MWYrKp2PK4SygC9ldzCtCnGf&s=1","manifestURL":"","manifest":null,"evtType":"openwindow"} 
I/GeckoConsole( 5063): Content JS LOG: XXX We have an iframe 

Each case seems to be handled identically, so I assume that something in the actual content of the page is triggering this behaviour(?)
(Assignee)

Comment 22

3 years ago
So, interestingly, the first link has a meta viewport and the second doesn't - could be we're not handling opened windows without an explicit viewport correctly?
Is it hitting http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#7673 ?
(Assignee)

Comment 24

3 years ago
For this second page, Gecko is under the impression that it has a viewport when it doesn't - putting debug output in content/base/src/nsDocument.cpp nsDocument::GetViewportInfo, you can see that it takes the DisplayWidthHeightNoZoom case.

I'm not sure where it's getting this from, my assumption would be that it gets it from the Mobile facebook link redirector and the document gets reused on redirection, whereby the viewport isn't reset to Unknown and it carries on getting used (unless the page specifies another meta viewport).

This appears to be a platform problem, changing component to reflect.
(Assignee)

Updated

3 years ago
Component: Gaia::System::Window Mgmt → Document Navigation
Product: Firefox OS → Core
(Assignee)

Comment 25

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #23)
> Is it hitting
> http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.
> cpp#7673 ?

This does get hit at some point, but given that the page isn't zoomable, I'm assuming that it must be for some other frame... Need to add some more detailed logging.
(In reply to Chris Lord [:cwiiis] from comment #25)
> This does get hit at some point, but given that the page isn't zoomable, I'm
> assuming that it must be for some other frame... Need to add some more
> detailed logging.

If it's hitting that that means the page is being treated as an "app" rather than a "browser", and it has no meta-viewport tag, so we give it a default meta-viewport tag that is not zoomable [1]. This was done for backwards compatibility. Gaia should not be opening it as an "app", it should be opening it as a "browser", whatever that means.

[1] http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp?rev=78c4d7d788d8#7678
(Assignee)

Comment 27

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #23)
> Is it hitting
> http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.
> cpp#7673 ?

I put my logging in the wrong place - it is hitting that :/

I'm not sure what the correct fix would be here to be honest, if we want that work-around in place, then I'm not sure how we can differentiate between a window.open opening something related to the 'app' a window.open to launch a link...

Given that until bug 1060191 these new windows would've gotten entirely new wrappers though, maybe we should disable this IsApp for anything except the original window?
Given https://bugzilla.mozilla.org/show_bug.cgi?id=940036#c22 I'm happy to take out this code now (and in 2.1) so that we use the page-specified viewport tag. That should fix this issue.
(Note however that there might be other problems caused by these links opening in "app" windows which might still need to be addressed, unless that aspect of the change was intentional)
(Assignee)

Comment 30

3 years ago
Created attachment 8509674 [details] [diff] [review]
Remove b2g app meta-viewport work-around

Excellent, let's get rid of this (confirmed fixes the problem)
Attachment #8509674 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 31

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #29)
> (Note however that there might be other problems caused by these links
> opening in "app" windows which might still need to be addressed, unless that
> aspect of the change was intentional)

This bug only affects the Facebook app, which is a shallow package of their mobile website (so it is indeed an app). I suppose they shouldn't be using window.open to open external links in an app (though I wonder if it's expected behaviour that a domain change in a new window still counts as being in the app?)

That's a separate issue anyway.
Attachment #8509674 - Flags: review?(bugmail.mozilla) → review+
(In reply to Chris Lord [:cwiiis] from comment #31)
> I suppose they shouldn't be using
> window.open to open external links in an app (though I wonder if it's
> expected behaviour that a domain change in a new window still counts as
> being in the app?)

I don't know if Fennec webapps are comparable, but there when I have a link that does target="_blank" (which should be equivalent to window.open), it spawns the link in the browser outside the app. If I click on a link to another domain it will load that page in the webapp window directly but have a little statusbar thingy at the top that indicates the domain of the page I'm on. Personally I would expect window.open here to not open the link inside the same app.
(Assignee)

Comment 33

3 years ago
Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/27624d9e8f27
(Assignee)

Comment 34

3 years ago
n?alive to comment on comment #32, as it's window-management related. I'm inclined to agree. afaik, that link handling is done Gecko-side though.
Flags: needinfo?(alive)
Backed out for B2G reftest failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7cc0208122f

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3223963&repo=mozilla-inbound
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3224196&repo=mozilla-inbound

The above are just what's failed so far. There could be more. Please verify green on Try.
And mochitest failures:
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3221951&repo=mozilla-inbound
(In reply to Chris Lord [:cwiiis] from comment #31)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #29)
> > (Note however that there might be other problems caused by these links
> > opening in "app" windows which might still need to be addressed, unless that
> > aspect of the change was intentional)
> 
> This bug only affects the Facebook app, which is a shallow package of their
> mobile website (so it is indeed an app). I suppose they shouldn't be using
> window.open to open external links in an app (though I wonder if it's
> expected behaviour that a domain change in a new window still counts as
> being in the app?)
> 
> That's a separate issue anyway.

If I understand the problem correctly,
facebook is always using a "redirect" way to navigate a link;

That is to say, when you click a link to |www.google.com| in facebook app, it will call window.open('http://facebook.com/?redirect=www.google.com').

Before the redirecting happens, the opened window should be considered to be part of the facebook - argee?
But after the page is redirected outside the origin of facebook to |www.google.com|,
we should not treat if it's still inside facebook, but we need to:
* Fix bug 1060191 which is pointed out by Vivien.
* Tell gecko that the page now is not part of the app but more like a webpage.

I wonder if gecko needs gaia to "remove mozApp attribute" to let it know the mozbrowser iframe now becomes a webpage from an app? Do we support that? Or gecko is smart enough to deal with this case...
Flags: needinfo?(alive)
Ah, I didn't realize it was a redirect; that makes more sense then. I don't know if gecko supports removing the mozapp attribute on a window already created, but it does sound like that would be the correct solution here. We should file a follow-up bug to track this outstanding issue.
(Assignee)

Comment 39

3 years ago
Meanwhile, I assume all the test failures for this are because we have test pages that need viewport tags - I should've anticipated this, sorry to the sheriffs. Will make sure the next is green before pushing.
(Assignee)

Comment 40

3 years ago
Tbpl link for convenience: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=27624d9e8f27
Status: NEW → ASSIGNED
(Assignee)

Comment 41

3 years ago
Created attachment 8511970 [details] [diff] [review]
Remove b2g app meta-viewport work-around + test fixes

I'm carrying the r+, but unfortunately, this patch still has one failing reftest:

http://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=http://ftp.mozilla.org/pub/mozilla.org/b2g/try-builds/chrislord.net@gmail.com-c802f3bdcb8c/try-emulator/try_ubuntu64_vm-b2g-emulator_test-reftest-11-bm117-tests1-linux64-build250.txt.gz&only_show_unexpected=1

Looking at the screen, this seems to hint at some other bug that this exposes, rather than the reftest actually failing (the area of the test matches exactly).
Attachment #8509674 - Attachment is obsolete: true
Attachment #8511970 - Flags: review+
(Assignee)

Comment 42

3 years ago
At this point, this is getting well outside my area of knowledge and I'm not sure what to investigate next.

n?milan to hand this off to someone on gfx team (though perhaps it's actually a layout problem?)
Assignee: chrislord.net → nobody
Flags: needinfo?(milan)
(Assignee)

Comment 43

3 years ago
This was the latest try push, for reference: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c802f3bdcb8c
Whiteboard: [systemsfe]
So, to be clear, this is the right fix, but we have failing tests, and that's the "only" issue?
Flags: needinfo?(milan)
(Assignee)

Comment 45

3 years ago
(In reply to Milan Sreckovic [:milan] from comment #44)
> So, to be clear, this is the right fix, but we have failing tests, and
> that's the "only" issue?

That's correct.
Depends on: 1089822
(Assignee)

Comment 46

3 years ago
I'm not going to assign this to myself just yet, but I'm trying to work around the bug that this seems to expose:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=145b3ff4f860
Status: ASSIGNED → NEW
(Assignee)

Comment 47

3 years ago
Who am I kidding... Patch incoming.
Assignee: nobody → chrislord.net
Status: NEW → ASSIGNED
(Assignee)

Comment 48

3 years ago
Created attachment 8516799 [details] [diff] [review]
Remove b2g app meta-viewport work-around + test fixes v2

Carring r=kats, asking r?roc for the change to layout/reftests/image/background-image-zoom-1-*

I don't believe my change affects what the test is trying to achieve, but it should be reviewed nonetheless.

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e880faddd4f7
Attachment #8511970 - Attachment is obsolete: true
Attachment #8516799 - Flags: review?(roc)
Attachment #8516799 - Flags: review?(bugmail.mozilla)
(Assignee)

Updated

3 years ago
Attachment #8516799 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8516799 [details] [diff] [review]
Remove b2g app meta-viewport work-around + test fixes v2

Review of attachment 8516799 [details] [diff] [review]:
-----------------------------------------------------------------

Looks OK but are we really confident that FxOS apps have <meta viewport>s of their own now?
Attachment #8516799 - Flags: review?(roc) → review+
(Assignee)

Comment 50

3 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #49)
> Comment on attachment 8516799 [details] [diff] [review]
> Remove b2g app meta-viewport work-around + test fixes v2
> 
> Review of attachment 8516799 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks OK but are we really confident that FxOS apps have <meta viewport>s of
> their own now?

I've been running with this patch for a week or two and haven't come across any apps that don't - we've given plenty of warning, and our behaviour here is the stand-out vs. every other mobile browser, so I think it's reasonable to make this change now.
(Assignee)

Comment 51

3 years ago
Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/dffbf0a55955
https://hg.mozilla.org/mozilla-central/rev/dffbf0a55955
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: 2.1 S7 (24Oct) → 2.1 S8 (7Nov)
(Assignee)

Comment 53

3 years ago
Comment on attachment 8516799 [details] [diff] [review]
Remove b2g app meta-viewport work-around + test fixes v2

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug
User impact if declined: Very poor usability on most links opened from an app
Testing completed: Tested on dog-food phone for a week or two, passes tests
Risk to taking this patch (and alternatives if risky): Risk of causing some apps to appear zoomed out if they don't adhere to standards (this fixes a long-standing b2g behaviour oddity that apps could have relied on, but that we warned we'd remove a long time ago)
String or UUID changes made by this patch: None
Attachment #8516799 - Flags: approval-mozilla-b2g34?
Issue verified fixed on Flame 2.2

Actual Results: User can zoom in and out of webpage opened via link from Facebook app.

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141105160209
Gaia: 7918024c737c4570cacd784f267e28737ae05dea
Gecko: 2114ef80f6ae
Gonk: 
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0


Leaving verifyme keyword for uplift to 2.1
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
status-b2g-v2.2: affected → verified
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
(In reply to Brogan Zumwalt [:BroganZ] from comment #54)
> Issue verified fixed on Flame 2.2
> 
> Actual Results: User can zoom in and out of webpage opened via link from
> Facebook app.
> 
> Device: Flame 2.2 Master (319mb)(Kitkat Base)(Shallow Flash)
> BuildID: 20141105160209
> Gaia: 7918024c737c4570cacd784f267e28737ae05dea
> Gecko: 2114ef80f6ae
> Gonk: 
> Version: 36.0a1 (2.2 Master)
> Firmware: V188-1
> User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
> 
> 
> Leaving verifyme keyword for uplift to 2.1

I am approving for uplift but Brogan, given he highlighted risk, can we also try testing some other commonly used apps here ?

Updated

3 years ago
Flags: needinfo?(bzumwalt)

Updated

3 years ago
Attachment #8516799 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/62a455f806ea
status-b2g-v2.1: affected → fixed
status-firefox34: --- → wontfix
status-firefox35: --- → wontfix
status-firefox36: --- → fixed
Whiteboard: [systemsfe]
Links to zoomable pages via Marketplace downloaded apps Twitter, SoundCloud, and YouTube show as expected. User can zoom in and out on all pages tested.
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(bzumwalt) → needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Issue verified fixed on Flame 2.1

Actual Results: User can zoom in and out of webpage opened via link from Facebook app.

Device: Flame 2.1(319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141110001201
Gaia: 0ec1925fc37b7c71d129ae44e42516a0cfb013c4
Gecko: 97487a2d1ee6
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
status-b2g-v2.1: fixed → verified
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.