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)

ARM
Android
defect

Tracking

(firefox24 wontfix, firefox25 wontfix, firefox26 wontfix, firefox27 affected, firefox28 affected, firefox29 affected, fennec+)

RESOLVED FIXED
Tracking Status
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)

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.
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.
Status: UNCONFIRMED → NEW
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
I'm also seeing content jitter and lock up on scroll with the dynamic toolbar disabled as well
tracking-fennec: --- → ?
Assignee: nobody → bugmail.mozilla
tracking-fennec: ? → 24+
Status: NEW → ASSIGNED
Kats? ping?
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.
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.
Attached patch Patch (deleted) — Splinter Review
Attachment #811191 - Flags: review?(wjohnston)
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+
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.
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.
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.
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.
And obviously with some debug logging it passes just fine. https://tbpl.mozilla.org/?tree=Try&rev=a8369d46013e
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
tracking-fennec: 24+ → ?
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
tracking-fennec: ? → +
Assignee: bugmail.mozilla → nobody
Status: ASSIGNED → NEW
Whiteboard: [mentor=kats][lang=js][bad first bug]
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.
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]
Cheers! :)
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.
filter on [mass-p5]
Priority: -- → P5
Assignee: bugmail.mozilla → nobody
This is fixed with bug 1180295.
Status: NEW → RESOLVED
Closed: 9 years ago
Depends on: dynamic-toolbar-2
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: