Closed
Bug 911152
Opened 11 years ago
Closed 9 years ago
The Dissolve website can not be scrolled
Categories
(Firefox for Android Graveyard :: Toolbar, defect, P5)
Tracking
(firefox24 wontfix, firefox25 wontfix, firefox26 wontfix, firefox27 affected, firefox28 affected, firefox29 affected, fennec+)
People
(Reporter: timbugzilla, Unassigned)
References
()
Details
(Keywords: regression, reproducible)
Attachments
(1 file)
(deleted),
patch
|
wesj
:
review+
kats
:
checkin-
|
Details | Diff | Splinter Review |
Firefox 24 beta will not load "The Dissolve" website (this is a popular new film website from Pitchfork.com and the top writers from the AV Club).
The site will partially load, but not completely, and cannot be scrolled etc. It will also greatly slow down or stop subsequent attempts to load other websites.
I am using a Nexus 4 with stock android 4.3 (Build JWR99Y).
I do have a custom HOSTS file (via AdFree Android), but this does not stop other browsers on the device loading the site.
Comment 1•11 years ago
|
||
The site loads for me and I can definitely reproduce the scrolling issue; content is seemingly locked in position.
I've confirmed that disabling the dynamic-toolbar seems to resolve this issue (about:config → browser.chrome.dynamictoolbar; false). I've CC'ed Kats and Chris to take a look.
Blocks: dynamic-toolbar
Status: UNCONFIRMED → NEW
status-firefox24:
--- → affected
status-firefox25:
--- → affected
status-firefox26:
--- → affected
Component: General → Graphics, Panning and Zooming
Ever confirmed: true
Summary: Firefox 24 beta fails to load The Dissolve website → The Dissolve website can not be scrolled
Version: Firefox 24 → Trunk
Comment 2•11 years ago
|
||
I'm also seeing content jitter and lock up on scroll with the dynamic toolbar disabled as well
Updated•11 years ago
|
tracking-fennec: --- → ?
Updated•11 years ago
|
Keywords: regression,
reproducible
Updated•11 years ago
|
Assignee: nobody → bugmail.mozilla
tracking-fennec: ? → 24+
Updated•11 years ago
|
Status: NEW → ASSIGNED
Comment 3•11 years ago
|
||
Kats? ping?
Comment 4•11 years ago
|
||
I took a quick look at the behaviour and it's definitely pretty weird. I don't know offhand where the problem is - will need more detailed investigation.
Comment 5•11 years ago
|
||
I looked into this a little bit. It appears that JS code on the site is not playing well with our dynamic toolbar. On desktop with the UA switcher and responsive design view, every time I resize the window area, the page adds another meta-viewport tag (with the exact same content). This is probably a bug in the page JS, but by itself it would be relatively harmless. The problem is that setting the meta-viewport tag triggers some of our dynamic toolbar code to recalculate some stuff, which then turns around and dispatches another resize event back to the page (I think). This causes an infinite cycle of meta-viewport tags being added and resizes, even though nothing on the page is actually changing. It shouldn't be too hard to break the cycle by caching something somewhere.
Comment 6•11 years ago
|
||
Attachment #811191 -
Flags: review?(wjohnston)
Updated•11 years ago
|
status-firefox27:
--- → affected
Comment 7•11 years ago
|
||
Comment on attachment 811191 [details] [diff] [review]
Patch
Review of attachment 811191 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/chrome/content/browser.js
@@ +3822,5 @@
>
> + if (aMetadata.equals(this.metadata)) {
> + // the metadata hasn't actually changed, so no need to do anything
> + return;
> + }
Move the comment outside this and drop the braces.
Attachment #811191 -
Flags: review?(wjohnston) → review+
Comment 8•11 years ago
|
||
Landed with review comment addressed:
https://hg.mozilla.org/integration/fx-team/rev/cfb6cd8accc3
Backed out in https://hg.mozilla.org/integration/fx-team/rev/cdcb01632607 along with bug 911574 for probably causing this robocop-1 orange: https://tbpl.mozilla.org/php/getParsedLog.php?id=28497023&tree=Fx-Team
Comment 10•11 years ago
|
||
The log above has this error:
18:55:02 INFO - 09-27 18:54:28.867 E/GeckoConsole( 3569): [JavaScript Error: "ReferenceError: minZoom is not defined" {file: "chrome://browser/content/browser.js" line: 5811}]
So it is probably this bug and not bug 911574 that caused the orange. Not really sure why minZoom wouldn't be defined though. I'll look into it.
Comment 11•11 years ago
|
||
According to the panda rc1 failure at https://tbpl.mozilla.org/?tree=Try&rev=423e0e0b910a it seems this error shows up when "aOther.minZoom" is reported to be null by JSON.stringify. The error doesn't show up if "aOther.minZoom" is 0.
I suspect this means that the prototype behaviour defined in browser.js for ViewportMetadata isn't having the intended effect.
Comment 12•11 years ago
|
||
Turns out my patch used "minZoom" instead of "this.minZoom" which probably caused the error. Try push with that fixed: https://tbpl.mozilla.org/?tree=Try&rev=1824806696b5 to verify.
Comment 13•11 years ago
|
||
The try build above still has one rc1 failure that I cannot explain :(
The JS error I saw previously is gone, but the same failure results (the long-click on testPictureLinkContextMenu won't find "Copy Image Location" in the context menu for some reason) and there's no indication of what went wrong in the logcat.
Comment 14•11 years ago
|
||
And obviously with some debug logging it passes just fine.
https://tbpl.mozilla.org/?tree=Try&rev=a8369d46013e
Comment 15•11 years ago
|
||
A little bit of progress. With the debug logging on a successful run I see the long press get triggered on the HTMLImageElement [1] but on an unsuccessful run I see the long press get triggered on the HTMLDivElement [2] that contains the anchor that contains the image. That explains why the context menu items don't show up. Not sure what happens to the image, it would help to have a screenshot. Let me try that.
(search for "XXX" in the logs below)
[1] https://tbpl.mozilla.org/php/getParsedLog.php?id=28754343&tree=Try&full=1
[2] https://tbpl.mozilla.org/php/getParsedLog.php?id=28754238&tree=Try&full=1
Updated•11 years ago
|
tracking-fennec: 24+ → ?
Comment 16•11 years ago
|
||
For the record I did do a try push [1] where I obtained the screenshots of what was happening. The screenshots showed that in some cases the page was getting laid out with the wrong CSS viewport (or the initial zoom was wrong) and I have no idea why. A race condition somewhere, probably, but I can't reproduce locally and debugging via try is just too painful for things like this.
[1] https://tbpl.mozilla.org/?tree=Try&rev=7260da5cf527
Updated•11 years ago
|
tracking-fennec: ? → +
Updated•11 years ago
|
Assignee: bugmail.mozilla → nobody
Status: ASSIGNED → NEW
status-firefox28:
--- → affected
status-firefox29:
--- → affected
Whiteboard: [mentor=kats][lang=js][bad first bug]
Reporter | ||
Comment 17•11 years ago
|
||
Dear All,
What is the status of this bug? It's been nearly 5 months and the site still cannot be scrolled in FF beta.
Comment 18•11 years ago
|
||
Thanks for poking! I took another look at the old patch I had for this bug and the problem was rather obvious: obtaining the old metadata via "this.metadata" will return a default ViewportMetadata instance if there is no pre-existing metadata. On a new page load this will often be equal to the aMetadata passed in, and my equals() check was causing the function to bail out when it shouldn't have. The fix is to ensure that when comparing the metadata objects, the old metadata actually exists. I'll do some try pushes to verify and update the patch.
Assignee: nobody → bugmail.mozilla
Whiteboard: [mentor=kats][lang=js][bad first bug]
Reporter | ||
Comment 19•11 years ago
|
||
Cheers! :)
Comment 20•11 years ago
|
||
Also it turns out that because allowDoubleTapZoom is now set in updateViewportSize (after the equals check happens) the equals check fails when we want it to pass. I'll have to put it somewhere else or find some other way to break the loop here.
Updated•10 years ago
|
Assignee: bugmail.mozilla → nobody
Updated•9 years ago
|
Attachment #811191 -
Flags: checkin-
Comment 22•9 years ago
|
||
This is fixed with bug 1180295.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•