Closed Bug 1026444 Opened 10 years ago Closed 9 years ago

Social's error page does not work in the panel view

Categories

(Firefox Graveyard :: SocialAPI, defect)

30 Branch
defect
Not set
normal

Tracking

(firefox34 wontfix, firefox35 wontfix, firefox36 verified, firefox37 verified)

VERIFIED FIXED
Firefox 37
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- verified
firefox37 --- verified

People

(Reporter: standard8, Assigned: mixedpuppy)

Details

Attachments

(2 files, 1 obsolete file)

Discovered whilst working on bug 1011392. The social error page doesn't get displayed in the panel view when a provider is using the toolbar.

STR:

1) Install Talkilla (has an out of date cert, only accept temporarily) from https://talkilla.mozillalabs.com
2) Restart Firefox

Actual results => The standard browser error page.

Expected results => The Social page.

This seems to have been broken since the panel was introduced since FF 30.
Flags: firefox-backlog+
Whiteboard: p=3 [qa+]
Points: --- → 3
Flags: qe-verify+
Whiteboard: p=3 [qa+]
Attached patch handle error pages in panels (obsolete) — Splinter Review
Assignee: nobody → mixedpuppy
Attachment #8538111 - Flags: review?(mhammond)
Comment on attachment 8538111 [details] [diff] [review]
handle error pages in panels

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

::: browser/base/content/socialmarks.xml
@@ +215,5 @@
>          }
>          this.content.addEventListener("DOMContentLoaded", DOMContentLoaded, true);
> +        // Force a layout flush by calling .clientTop so that the docShell of
> +        // this frame is created for the error listener
> +        this.content.clientTop;

I wonder if we are better off moving this "hack" to the listener itself, seeing we've needed to do it in 2 places in just this patch?

@@ +227,5 @@
> +        <parameter name="aNotificationFrame"/>
> +        <body><![CDATA[
> +        if (!aNotificationFrame)
> +          return;
> +    

trailing whitespace ;)
Attachment #8538111 - Flags: review?(mhammond) → review+
updated patch with test fixes, carry forward r+
Attachment #8538111 - Attachment is obsolete: true
Attachment #8539473 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/d7ec07241b4e
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Iteration: --- → 37.2
Comment on attachment 8539473 [details] [diff] [review]
handle error pages in panels

Approval Request Comment
[Feature/regressing bug #]: socialapi panels
[User impact if declined]: some panels show standard error page which does not fit or appear correctly in the panels
[Describe test coverage new/current, TBPL]: tbpl
[Risks and why]: low, most of the change is updating mochitests
[String/UUID change made/needed]: none
Attachment #8539473 - Flags: approval-mozilla-beta?
Attachment #8539473 - Flags: approval-mozilla-aurora?
Comment on attachment 8539473 [details] [diff] [review]
handle error pages in panels

Since this has been around for a bit and it's later in the beta cycle, let's wait on beta and let this ride a bit with 36.
Attachment #8539473 - Flags: approval-mozilla-beta?
Attachment #8539473 - Flags: approval-mozilla-beta-
Attachment #8539473 - Flags: approval-mozilla-aurora?
Attachment #8539473 - Flags: approval-mozilla-aurora+
Talkilla is not available anymore, is there another SocialAPI provider I can use to test this?
Flags: needinfo?(standard8)
Shane had a test one that might be suitable
Flags: needinfo?(standard8) → needinfo?(mixedpuppy)
you should be able to test with any provider by going offline.  for simplicity sake, http://socialapi-demo.paas.allizom.org/
Flags: needinfo?(mixedpuppy)
Attached image Screenshot error
I an not able to reproduce the initial issue, using an old build before the fix and latest Nightly will show the same error page 'Nightly is unable to connect with Demo Social Service right now' with options to 'Try again' and 'Close This Sidebar'

A few issues:
1. 'Close This Sidebar' does not work if I disconnect the internet cable.
2. The icons from Demo provider are invisible in the navigation bar.

Mark since you reported the issue can you see the fix in latest Beta and Aurora?
Shane what`s your opinion about the issues from above, worth logging bugs?
Flags: needinfo?(standard8)
Flags: needinfo?(mixedpuppy)
Oh, I suspect the offline page probably did work - its not an "error" page as such.

I think the error one we may have been on about here is the out of date certificate. That's going to be harder to reproduce - I haven't got a setup any more that would allow us to test that.
Flags: needinfo?(standard8)
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #15)
> Created attachment 8551729 [details]
> Screenshot error
> 
> I an not able to reproduce the initial issue, using an old build before the
> fix and latest Nightly will show the same error page 'Nightly is unable to
> connect with Demo Social Service right now' with options to 'Try again' and
> 'Close This Sidebar'
> 
> A few issues:
> 1. 'Close This Sidebar' does not work if I disconnect the internet cable.

that should probably be removed.

> 2. The icons from Demo provider are invisible in the navigation bar.

live providers use data urls for the icons so it's only a problem with the demo provider.  we should probably use the icon caching that is used for favicons to keep them around.  it would be a very low priority.

> Mark since you reported the issue can you see the fix in latest Beta and
> Aurora?
> Shane what`s your opinion about the issues from above, worth logging bugs?
Flags: needinfo?(mixedpuppy)
The patch I did added error page support to a number of panels that did not have that at all, iirc only the sidebar had properly working error page support.  The share panel, chat window and social bookmark button all did not have custom error page handling.
(In reply to Shane Caraveo (:mixedpuppy) from comment #18)
> The patch I did added error page support to a number of panels that did not
> have that at all, iirc only the sidebar had properly working error page
> support.  The share panel, chat window and social bookmark button all did
> not have custom error page handling.

I just checked the share panel, save page and Demo social service and I can reproduce the initial issue on old Nightlys and confirm that using Firefox 36 beta 2 and latest Aurora the issue is not reproducible anymore.

Regarding the 'Share panel', I see that if I enter 'Share more services' (the + button from 'Share panel') the old error page is there and not the new added one. Is this intended?
Flags: needinfo?(mixedpuppy)
I will go ahead and close this bug as Verified fixed, and if the potential issue from above turns out to be something that is not intended I will log a new bug on that.
Status: RESOLVED → VERIFIED
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #19)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #18)

> Regarding the 'Share panel', I see that if I enter 'Share more services'
> (the + button from 'Share panel') the old error page is there and not the
> new added one. Is this intended?

Hmm, I explicitly worked on that issue, make a new bug with STR for it.
Flags: needinfo?(mixedpuppy)
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.