Closed
Bug 1080953
Opened 10 years ago
Closed 9 years ago
UITour: tell page when first incoming call is received and if that room window is open
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
People
(Reporter: Dolske, Assigned: MattN)
References
Details
Attachments
(1 file, 1 obsolete file)
The Hello tour guides a user through creating their first Hello room and sharing that link. When the person receiving the link initiates a call (I.E. the tour user's first incoming call is received), the tour should resume. * If the tour page is still open, that tab should be selected. If the tour page is not open, a tab should be opened with the tour (and resumed at the proper point, mechanism TBD). * If the room view for the call was already open, the call connects and everything is obvious. But if the view isn't open, the tour needs to know so that it can open the Hello panel, indicate how to click an active room, and know when the user has opened the room so the tour can continue. [This bug needs to tell the tour if the incoming call's room is open, those other steps will be implemented by other bugs.] * Something will need to track tour status (probably the bug implementing the First Time Experience (FTE) panel for Hello), as we only want this bug to do things when finishing a tour. (So, a tristate flag: Tour not started, Tour waiting for incoming call, Tour Completed). * In particular, we'd like to avoid annoying QA/developers who are not interested in a tour, and don't want the first call in testing profiles to be resuming a tour they never started.
Assignee | ||
Comment 1•9 years ago
|
||
This is probably a lower priority for now as it's not part of the reduced-scope tour.
Comment 2•9 years ago
|
||
After reviewing the reduced-scope wire frames during the PDX work-week, it looks like this bug is still required as part of the overall FTE flow.
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Alex Gibson [:agibson] from comment #2) > After reviewing the reduced-scope wire frames during the PDX work-week, it > looks like this bug is still required as part of the overall FTE flow. IMO the wireframes from the PDX work week went back to the longer, mostly original, flow. This is my proposal: == Tour tab is closed when someone else joins a room for the first time after opening the tour == * no "incomingConversation" query parameter – User is opening the tour to start from the beginning. * &incomingConversation=waiting – First incoming conversation after opening the tour and the conversation window is closed. Point to the conversation list to tell the user to open the conversation. * &incomingConversation=open – First incoming conversation after opening the tour and the conversation window is already open. Don't show room list. Instead go to success page (end of tour). == Tour tab is opened when someone else joins a room for the first time after opening the tour == One of the following will happen: * "Loop:IncomingConversation" notification with details `conversationOpen: false` * "Loop:IncomingConversation" notification with details `conversationOpen: true` To clarify, the page will either get the query parameter with a page load in a new tab (if the URL wasn't open) XOR the event will be fired at the page.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Iteration: --- → 37.2
Points: --- → 8
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox37:
--- → affected
Flags: qe-verify-
Flags: firefox-backlog+
Assignee | ||
Comment 4•9 years ago
|
||
Assignee | ||
Comment 5•9 years ago
|
||
/r/1421 - Bug 1080953 - UITour: tell page when first incoming call is received and if that room window is open. Pull down this commit: hg pull review -r a83c24ee066adc35197be5d6c117db2904b46ba0
Assignee | ||
Comment 6•9 years ago
|
||
What should we use for the `utm_campaign` query parameter when we resume the tour from the first conversation but the user had already closed the existing tour tab? My proposal was "resume-with-conversation"
Flags: needinfo?(cprice)
Comment 7•9 years ago
|
||
To be clear, you are referring to the 2nd row in the wireframe (http://cl.ly/image/3j2w0i0T0c1b) - "User clicks 'active' Hello icon in chrome", correct? The value for utm_campaign was "getting-started" from the button (bug 1099462), and "settings-menu" from the gear menu (bug 1101286). I recommend keeping with the convention of tagging the source, and use "hello-icon-resume". Thanks
Flags: needinfo?(cprice)
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to Cory Price [:ckprice] from comment #7) > To be clear, you are referring to the 2nd row in the wireframe > (http://cl.ly/image/3j2w0i0T0c1b) - "User clicks 'active' Hello icon in > chrome", correct? There isn't a single source/element entry point for opening a new tab: * The user isn't in the room when someone else joins for the first time. We re-open the tab when the hello panel is opened next. * The user is in the room when someone else joins for the first time. We open the tab immediately.
Comment 9•9 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #8) > There isn't a single source/element entry point for opening a new tab: > * The user isn't in the room when someone else joins for the first time. We > re-open the tab when the hello panel is opened next. > * The user is in the room when someone else joins for the first time. We > open the tab immediately. For the purposes of this bug, "resume-with-conversation" will be sufficient. The wireframe shows two different states depending on if the initiating user has the room open already vs if the initiating user has closed the room/tab and it has to be re-opened via the Hello icon. If the URL will be the same for these scenarios, we'll need to ensure we can detect which state to raise some other way. I will continue this conversation in bug 1109132. Thanks
Assignee | ||
Updated•9 years ago
|
Attachment #8535428 -
Flags: review?(standard8)
Attachment #8535428 -
Flags: review?(bmcbride)
Assignee | ||
Comment 10•9 years ago
|
||
/r/1421 - Bug 1080953 - UITour: tell page when first incoming call is received and if that room window is open. Pull down this commit: hg pull review -r d5a1089c742ae409b51dbed0a2c516a81393e79e
Assignee | ||
Comment 11•9 years ago
|
||
I didn't have time to make tests yet but there is pressure to get this landed ASAP so I'm putting the patch up for review without tests.
Assignee | ||
Comment 12•9 years ago
|
||
Try push: https://tbpl.mozilla.org/?tree=Try&rev=60b022dc08b3 Alex, you can test out this patch with the downloads from https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mozilla@noorenberghe.ca-60b022dc08b3/ in a few hours.
Flags: needinfo?(agibson)
Comment 13•9 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #12) > Try push: https://tbpl.mozilla.org/?tree=Try&rev=60b022dc08b3 > > Alex, you can test out this patch with the downloads from > https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ > mozilla@noorenberghe.ca-60b022dc08b3/ in a few hours. Thanks, Matt!
Flags: needinfo?(agibson)
Comment 14•9 years ago
|
||
https://reviewboard.mozilla.org/r/1419/#review927 ::: browser/base/content/browser-loop.js (Diff revision 2) > + let roomsWithNonOwners = yield new Promise(resolve => { I think this chunk wants to be its own function for ease of unit testing. That said, if you'd prefer to make that change when you actually write the tests, that's fine. ::: browser/base/content/browser-loop.js (Diff revision 2) > + }).then(Task.async(function* () { This has made openCallPanel get really giant, to the point where a reader or maintainer is required to carry a lot of mental state. If it's easy, I'd propose hoisting the anonymous function here into it's own non-anonymous function, but to ease that problem, and to improve testability. ::: browser/components/loop/MozLoopService.jsm (Diff revision 2) > + // The particpant that joined isn't necessarily included in room.participants so concat It would be nice if this comment explain how that's possible. ::: browser/components/loop/MozLoopService.jsm (Diff revision 2) > + let hadExistingTab = win.switchToTabHavingURI(url, true, { A comment explaining why the ignore stuff is in use would be helpful. ::: browser/components/loop/MozLoopService.jsm (Diff revision 2) > + conversationOpen: aIncomingConversationState == "open", nit: ===
Updated•9 years ago
|
Attachment #8535428 -
Flags: review+
Comment 15•9 years ago
|
||
https://reviewboard.mozilla.org/r/1419/#review929 Ship It!
Comment 16•9 years ago
|
||
https://reviewboard.mozilla.org/r/1419/#review931 Looks good; r=dmose with the various bits addressed.
Updated•9 years ago
|
Attachment #8535428 -
Flags: review?(standard8)
Attachment #8535428 -
Flags: review?(bmcbride)
Comment 17•9 years ago
|
||
(In reply to Dan Mosedale (:dmose) - use needinfo? (not reading bugmail regularly) from comment #14) > > This has made openCallPanel get really giant, to the point where a reader or > maintainer is required to carry a lot of mental state. If it's easy, I'd > propose hoisting the anonymous function here into it's own non-anonymous > function, but to ease that problem, and to improve testability. "both to ease that problem".
Comment 18•9 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #3) > == Tour tab is opened when someone else joins a room for the first time > after opening the tour == > One of the following will happen: > * "Loop:IncomingConversation" notification with details `conversationOpen: > false` > * "Loop:IncomingConversation" notification with details `conversationOpen: > true` Testing on the build in Comment 12, I'm not seeing the "Loop:IncomingConversation" event fire (along with details `conversationOpen: false`) if I have closed the rooms view. Nothing happens in this case, until I open the hello panel and click to re-open the conversation. At this point, I get a ""Loop:IncomingConversation" event with the details `conversationOpen: true`?
Comment 19•9 years ago
|
||
(In reply to Alex Gibson [:agibson] from comment #18) > (In reply to Matthew N. [:MattN] from comment #3) > > == Tour tab is opened when someone else joins a room for the first time > > after opening the tour == > > One of the following will happen: > > * "Loop:IncomingConversation" notification with details `conversationOpen: > > false` > > * "Loop:IncomingConversation" notification with details `conversationOpen: > > true` > > Testing on the build in Comment 12, I'm not seeing the > "Loop:IncomingConversation" event fire (along with details > `conversationOpen: false`) if I have closed the rooms view. Nothing happens > in this case, until I open the hello panel and click to re-open the > conversation. At this point, I get a ""Loop:IncomingConversation" event with > the details `conversationOpen: true`? Further to this, I'm not seeing the event regardless to if the tab is or isn't visible when the other person connects (if that helps at all).
Comment 20•9 years ago
|
||
I just also got a `conversationOpen: true` event even though the conversation view wasn't open, which resulted in the page going into the wrong state?
Comment 21•9 years ago
|
||
After some investigation it seems the event with `conversationOpen: false` will fire when clicking the Hello icon, but *only* if I access the page from either the "Get Started" button or the "Tour" link. If I refresh the page to take the tour again, it no longer seems to work. Is this the intended behavior? It seems inconsistent to if I leave the conversation view open, which behaves differently. Looking at the code, I assume this is due to `gettingStarted.resumeOnFirstJoin` being only set when first loading the tab? I still have no idea how I got an event with `conversationOpen: true` when the view was not open.
Comment 22•9 years ago
|
||
Sorry for all the comments, but I notice another oddity: If I try the tour several times by going through the full flow in a single session, the browser seems to open progressively more and more tabs each time it reconnects the conversation, e.g. the following URL got opened 4 times each in a separate tab: http://127.0.0.1:8000/en-US/firefox/37.0a1/hello/start/?incomingConversation=open&utm_source=firefox-browser&utm_medium=firefox-browser&utm_campaign=resume-with-conversation
Assignee | ||
Comment 23•9 years ago
|
||
Alex and I sorted out the issues and solutions in our weekly dev. meeting. * Part of the problem was misunderstandings/miscommunication on how the events work. * I had a TODO about moving the pref setting to another time to avoid nagging users who bail before the email/copy step but figured it could wait. Alex points out in comment 21 that setting the pref from the tour entrypoints can cause problems if someone visits the tour via a link shared from a friend or if they reload the tab and want the tour to continue. ** I will allow the page to set the pref when it wants (probably after email/copy are clicked) * I will look into the issue with multiple tabs opening
Assignee | ||
Updated•9 years ago
|
Attachment #8535428 -
Flags: review+
Assignee | ||
Comment 24•9 years ago
|
||
/r/1421 - Bug 1080953 - UITour: tell page when first incoming call is received and if that room window is open. r=dmose,dolske Pull down this commit: hg pull review -r d7500ef2a0921ad0d5d3a9e35bd8f80e8436d289
Assignee | ||
Updated•9 years ago
|
Attachment #8535428 -
Flags: review?(dolske)
Assignee | ||
Comment 25•9 years ago
|
||
/r/1421 - Bug 1080953 - UITour: tell page when first incoming call is received and if that room window is open. r=dmose,dolske Pull down this commit: hg pull review -r d7500ef2a0921ad0d5d3a9e35bd8f80e8436d289
Assignee | ||
Comment 26•9 years ago
|
||
I'm writing some quick tests now. I wasn't able to repro the multiple tabs opening yet. It would be good to know exactly which of the cases that happened in (e.g. conversation open/closed in advance and the entry point that lead to the tab opening).
Assignee | ||
Comment 27•9 years ago
|
||
/r/1421 - Bug 1080953 - UITour: tell page when first incoming call is received and if that room window is open. r=dmose,dolske Pull down this commit: hg pull review -r 1f2a1bc6dc11be5c0b2945bb9899d08654367287
Reporter | ||
Updated•9 years ago
|
Attachment #8535428 -
Flags: review?(dolske) → review+
Reporter | ||
Comment 28•9 years ago
|
||
https://reviewboard.mozilla.org/r/1419/#review957 Ship It!
Reporter | ||
Comment 29•9 years ago
|
||
Comment on attachment 8535428 [details]
MozReview Request: bz://1080953/MattN
[Triage Comment]
Needed for Fx35 Hello tour. No unusual risk.
Attachment #8535428 -
Flags: approval-mozilla-beta+
Attachment #8535428 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 30•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/8a4a29b0c603
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 31•9 years ago
|
||
Alex, you can download an updated build of what landed plus bug 1112565 at http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-macosx64/1418877969/ since I'm not sure if it will make it in today's Nightly.
Comment 32•9 years ago
|
||
On(In reply to Matthew N. [:MattN] from comment #31) > Alex, you can download an updated build of what landed plus bug 1112565 at > http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team- > macosx64/1418877969/ since I'm not sure if it will make it in today's > Nightly. An issues notice with the latest changes: - I leave the tab open after sharing the link, but close the conversation view: When the other connects, the hello icon turns blue as expected. I go to click it, but the hello panel doesn't open. At this point I get sent *lots* of "Loop:IncomingConversation" events to the point where it starts to slow down the browser. This also seems to effect all scenarios where the conversation connects. The next time the hello icon is clicked, I get hundreds of events one after the other?
Comment 33•9 years ago
|
||
Also I wonder if there's a risk of making the hello panel sticky automatically when the tab re-opens - if the web page doesn't land in a state where the panel can be dismissed again (e.g. the conversation connects successfully), then could the user be stuck with a panel they can't close?
Comment 34•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8a4a29b0c603
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 37
Comment 35•9 years ago
|
||
Filed Bug 1113247 as a follow up to some possible issues mentioned in Comment 32
Assignee | ||
Comment 36•9 years ago
|
||
(In reply to Alex Gibson [:agibson] from comment #33) > Also I wonder if there's a risk of making the hello panel sticky > automatically when the tab re-opens - if the web page doesn't land in a > state where the panel can be dismissed again (e.g. the conversation connects > successfully), then could the user be stuck with a panel they can't close? I'm not sure I understand but the page doesn't need to call hideMenu("loop") anymore since bug 1112565 as it will close on its own before switching tabs so I'm not sure how the hello panel would stay sticky. I'm looking into why clicking the hello button after a showMenu("loop") doesn't allow manually closing the panel like it does for "appMenu".
Comment 37•9 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #36) > I'm not sure I understand but the page doesn't need to call hideMenu("loop") > anymore since bug 1112565 as it will close on its own before switching tabs > so I'm not sure how the hello panel would stay sticky. I'm looking into why > clicking the hello button after a showMenu("loop") doesn't allow manually > closing the panel like it does for "appMenu". The page isn't calling hideMenu("loop") on load. I think I was hitting a side effect of where I was going wrong in Bug 1113247. It was causing the hello button to behave incorrectly. But it's a good point that showMenu("loop") does seem to prevent the user from closing the panel again.
Assignee | ||
Comment 38•9 years ago
|
||
(In reply to Alex Gibson [:agibson] from comment #37) > But it's a good point that showMenu("loop") does seem to prevent the user from closing the panel again. It's not hard to fix but I'm not sure it's worth it since switching away from a tour tab will also dismiss the panel. This is only an issue with UITour since normally clicking outside the panel (on its anchor) would dismiss the panel anyways.
Updated•9 years ago
|
Assignee | ||
Comment 40•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/e1e0db401960
Assignee | ||
Comment 41•9 years ago
|
||
Attachment #8535428 -
Attachment is obsolete: true
Attachment #8618402 -
Flags: review+
Assignee | ||
Comment 42•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•