994146 - Loop client should reset panel when activated

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Adam Roach [:abr]]

Currently, when one clicks on the Loop icon (presently, a phone handset), the drop-down panel state is whatever was last in that panel. While this is probably the correct behavior once we have an address book to display, right now, it leads to some odd behaviors:

1) If a URL was generated, then dropping down the box again will display the same URL. This runs counter to the intention that these URLs should be individually generated.
2) If an error was generated (e.g., "could not generate URL"), and the window is closed, that error is still displayed.

I assume there is some event we can intercept (either when the pane becomes invisible, or when it is about to become visible) that would allow us to reset it to its initial state. We need to add this functionality before MLP.

Comment 1

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

I'm not aware of such an event to listen to and I can't find any doc about it, but :mixedpuppy might know.

Comment 2

[Mark Banner (:standard8)]

Haven't we got some sort of onshow event for the document?

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

socialFrameShow? http://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser-social.js#1309

Comment 4

[Shane Caraveo (:mixedpuppy)]

Yes, use socialFrameShow.  Visibility api should also work, but needs verification.

Comment 5

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: https://github.com/adamroach/gecko-dev/pull/19

This patch uses the DOM visibility API, which is implemented in Desktop and works fairly great.

dmose, please take over the patch if you want DOM integration tests, I felt like unit tests & stubs were enough here.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8413845 [details] [review]
https://github.com/adamroach/gecko-dev/pull/19

r=dmose. Review notes and requests in the PR; please address as appropriate and squash before merging.

Thanks for the patch!

Comment 7

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Landed on loop-ui-initial with r=dmose: https://github.com/adamroach/gecko-dev/commit/e12cdb6a6285ecbb777d4e7893666328ddb6bc65

Comment 8

[Mark Banner (:standard8)]

https://hg.mozilla.org/projects/elm/rev/27a1e9458719

Comment 9

[Mark Banner (:standard8)]

https://hg.mozilla.org/mozilla-central/rev/27a1e9458719

Comment 10

[u279076]

Verified fixed through basic dogfooding. Adam, do you think this is worth having a regression test in Moztrap?

Comment 11

[Adam Roach [:abr]]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #10)
> Verified fixed through basic dogfooding. Adam, do you think this is worth
> having a regression test in Moztrap?

Short version: not yet.

Full version: it's going to be kind of tricky to put together a regression test will last very long, since this behavior will shortly change so that the URL changes only if it's been copied or shared. Long-term, we're going to want to test that "the right thing" happens when the panel opens, but I would hold off until we have the final UX implemented.

Comment 12

[u279076]

Thanks for the detailed response, Adam. Please needinfo me when you feel this needs to get a regression test.