812108 - The "Server not found" error page is displayed in the Social sidebar

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Ioana (away)]

Firefox 17.0 beta 5 - Build ID 20121106195758

STR:
1. Enable the Social sidebar in Firefox and log into Facebook.
2. Close Firefox. Close your internet connection.
3. Restart Firefox.
4. Click "Try again" on the sidebar. Click it again. (Leave the internet connection closed)

The "Server not found" error page is displayed in the sidebar. The sidebar should have preserved its UI until the error is gone - https://bugzilla.mozilla.org/attachment.cgi?id=681025.

Comment 1

[u279076]

I saw this today too but after I lost my connection.

1. Connected via VPN, Facebook Messenger active
2. Disconnect from VPN
> Facebook Messenger shows the Facebook "lost connection" UI
4. Connect to wireless network (not VPN)
5. Click "Try Again" in the sidebar
> Facebook Messenger displays the standard Firefox "Server Not Found" error page
6. Click "Try Again" on the error page in the sidebar
> Sidebar loads my activity feed but my Friends list is empty and the Facebook button shows me as logged out
7. Restart Firefox
> Facebook Messenger returns to working funtionality

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Attachment: Keep displaying the error message while the error persists.

The problem here is that both the sidebar and the frameworker failed (which is expected - the network is down) - but the sidebar error was reported after the frameworker - so the "try again" button is shown to reload just the sidebar.  When the user hits "try again", we attempt to reload the sidebar URL - which still fails - but as onLocationChange sees there is already a frameworker error it doesn't call setErrorMessage, so we get the "normal" error page.

The following patch arranges for a Social.errorState to "trump" a sidebar error - ie, as soon as we have a .errorState the frameworker error page is shown, even if a subsequent "sidebar" error comes in.  Thus, onLocationChange unconditionally calls setErrorMessage knowing the "right thing" (tm) will happen.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 685946 [details] [diff] [review]
Keep displaying the error message while the error persists.

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

As discussed on IRC, let's keep the error type and override it to a frameworker error if the provider is in an error state

::: browser/base/content/browser-social.js
@@ -1068,5 @@
>    },
>  
>    onLocationChange: function SPL_onLocationChange(aWebProgress, aRequest, aLocation, aFlags) {
>      let failure = aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE;
> -    if (failure && Social.errorState != "frameworker-error") {

about this, I'll leave that up to you. I don't recall under which conditions this was needed, but maybe it's no longer necessary for some reason, and if you notice that's the case then we can remove it.
I'd like to see this gone for good too but if we want to uplift this patch for beta perhaps we should make this removal in a separate bug

Comment 4

[Alex Keybl [:akeybl]]

We'd accept a low risk uplift, but no need to track for upcoming releases.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Attachment: lighter touch, as requested

This much simpler patch also works.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a41fb508d89

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a41fb508d89

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a41fb508d89

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 686416 [details] [diff] [review]
lighter touch, as requested

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 766616
User impact if declined: on network connectivity issues, more than 1 attempt of clicking "try again" in social sidebar will fail to display the proper error page
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none

Comment 10

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3597fdf5ce60
https://hg.mozilla.org/releases/mozilla-beta/rev/dd3ad86cb111

Comment 11

[u279076]

Ioana, can you please verify this is fixed? It should be fixed in Firefox 18, 19, and 20.

Comment 12

[Mihai Morar, (:MihaiMorar)]

Attachment: Screen Shot

I could reproduce this issue useing STR from Comment 0 on:

Windows 7 x64:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0 (20121128060531)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20121130 Firefox/19.0 (20121130042014)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121130 Firefox/20.0 (20121130030747)

Ubuntu x32
Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/20100101 Firefox/18.0 (20121128060531)
Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/20121130 Firefox/19.0 (20121130042014)
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121130 Firefox/20.0 (20121130030747)

Please see the attachments added.

It look like all theese 6 builds are affected so please someone with permission set flags for FF 18 19 and 20.

Comment 13

[Mihai Morar, (:MihaiMorar)]

Attachment: Ubuntu Screenshot

Comment 14

[Mark Hammond [:markh] [:mhammond]]

It looks like your builds might be pre the fix landing?

Comment 15

[u279076]

Mario, this should be fixed in...
 * Firefox 20 as of 2012-11-30
 * Firefox 19 as of 2012-12-03
 * Firefox 18 as of 18.0b3

Can you please confirm?

Comment 16

[u279076]

By the way, candidate builds for 18.0b3 can be found here:
ftp://ftp.mozilla.org/pub/firefox/nightly/18.0b3-candidates/build1/

Comment 17

[Mihai Morar, (:MihaiMorar)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #15)
> Mario, this should be fixed in...
>  * Firefox 20 as of 2012-11-30
>  * Firefox 19 as of 2012-12-03
>  * Firefox 18 as of 18.0b3
> 
> Can you please confirm?

Verified again on latest Nightly, Aurora and Beta:

Windows7 x64
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0 (20121205060959)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20121206 Firefox/19.0
(20121206042015)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121206 Firefox/20.0
(20121206030737)

Ubuntu x32
Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/20100101 Firefox/18.0 (20121205060959)
Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/20121206 Firefox/19.0
(20121206042015)
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121206 Firefox/20.0
(20121206030737)

Mac OS 10.8
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:18.0) Gecko/20100101 Firefox/18.0 (20121205060959)

It looks like this issue is fixed for all builds verified. "Page cannot be displayed" message error is not displayed anymore.

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

This doesn't meet criteria for ESR branch, wontfixing for esr17.