605077 - Gmail Inbox not loading in standard view

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[ahoza]

Attachment: Screenshot

Device: Nokia N900
BuildID: Mozilla /5.0 (Maemo;Linux armv7l;rv:2.0b8pre)Gecko/20101017 Firefox/4.0b8pre Fennec /4.0b2pre 

Device: HTC Desire
BuildID: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b8pre) Gecko/20101017 Firefox/4.0b8pre Fennec /4.0b2pre 

Steps to reproduce:

1. Start Fennec.
2. Load gmail.com and enter credentials.

Expected results:
Gmail Inbox page is displayed.

Actual results: 
"This is taking longer than usual" is displayed (see atached screenshot)
If one chooses "basic HTML view" link in that page, Gmail Inbox loads.

Note: Gmail Inbox page loads properly, in standard view, when using native browser. This should happen with Fennec as well.

Comment 1

[Tuukka Tolvanen (sp3000)]

reproducible already on 20101015

Comment 2

[Tuukka Tolvanen (sp3000)]

regressed at central 5b8582013988->5fcb86d51691, mobile d46fdbdbe45f->d46fdbdbe45f (sic)

Comment 3

[Tuukka Tolvanen (sp3000)]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5b8582013988&tochange=5fcb86d51691

Comment 4

[Tuukka Tolvanen (sp3000)]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ed24c21e6497&tochange=5fcb86d51691 bug 581226 bug 561051

Comment 5

[Tuukka Tolvanen (sp3000)]

c5494ee56c47 was good

Comment 6

[Brandon Sterne (:bsterne)]

Can you put a breakpoint in nsDSURIContentListener::CheckFrameOptions and see where you return and what is returned?

Comment 7

[Pekka Vanhoja]

(In reply to comment #6)
> Can you put a breakpoint in nsDSURIContentListener::CheckFrameOptions and see
> where you return and what is returned?

After beginning to load gmail I get one return from http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDSURIContentListener.cpp#311 and then 4 returns http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDSURIContentListener.cpp#341.

Comment 8

[Pekka Vanhoja]

Attachment: Patch to fix gmail loading

I didn't quite get what is the essential difference here but when looking at the changes http://hg.mozilla.org/mozilla-central/rev/f5c0015afe0e and http://hg.mozilla.org/mozilla-central/rev/5fcb86d51691 I noticed the while-loop having a different form in the condition. With this "older" form of the condition gmail inbox gets loaded ok.

Comment 9

[Josh Matthews [:jdm]]

>-        while (NS_SUCCEEDED(curDocShellItem->GetParent(getter_AddRefs(parentDocShellItem)) &&
>-                            parentDocShellItem)) {

Here's the problem: the NS_SUCCEEDED call includes the && parentDocShellItem condition.  Guh.

Comment 10

[Brandon Sterne (:bsterne)]

Attachment: patch (per comment 9)

(In reply to comment #9)
> Here's the problem: the NS_SUCCEEDED call includes the && parentDocShellItem
> condition.  Guh.

Yep. Pekka, can you test this patch to see if it fixes the Gmail problem?

Comment 11

[Brandon Sterne (:bsterne)]

I'm curious why this only shows up on Fennec, but I suspect Josh is right and hopefully the patch fixes us there.

Comment 12

[Brandon Sterne (:bsterne)]

This is the right fix.  The condition is wrong for both Firefox and Fennec but we survive on Fennec because we hit the break after finding a chrome docshell.  Since we don't find such a chrome docshell on Fennec, we enter the while loop an additional time with parentDocShell == null so we can't find a topDoc and thus return false blocking the frame from loading.  Thanks for catching this.

http://hg.mozilla.org/mozilla-central/rev/2ccdeb31509b

Comment 13

[Brandon Sterne (:bsterne)]

(In reply to comment #12)
> This is the right fix.  The condition is wrong for both Firefox and Fennec but
> we survive on Fennec

Should have read "survive on Firefox".

Comment 14

[Pekka Vanhoja]

(In reply to comment #12)
> This is the right fix.  
> ...
> http://hg.mozilla.org/mozilla-central/rev/2ccdeb31509b

Yes, this works. Thanks.

Comment 15

[Ayan Shah[:ashah]]

Verified on Build:

Mozilla/5.0(Android; Linux armv7l; rv:2.0b8pre) Gecko/20101021 Firefox/4.0b8pre
Fennec/4.0b2pre

Comment 16

[Reed Loden [:reed]]

Comment on attachment 484695 [details] [diff] [review]
patch (per comment 9)

I assume we also need this on 1.9.2 since we just took the regressing patch.

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 484695 [details] [diff] [review]
patch (per comment 9)

Approved for 1.9.2.13, a=dveditz

Comment 18

[Reed Loden [:reed]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5cb659ed04a0