Notes after UX desktop and mobile Loop meetings: * The URL should be short enough to be easily copied through text selection or to be share by SMS. This means a maximum of 8 characters to identify the call as part of the full URL * The URL is generated on the server side * The URL has to be displayed as the user opens the panel - it is assumed that the call to get the URL from the server would be done before opening the panel for performance reasons and that the call parameters (expiration date, link generator name and invitation name) are sent to the server when the URL is copied (copy button or selection of the URL text in the text box)or shared. These parameters are stored on the server.

I'm confused. I thought we came away from the Toronto talks that we wanted to generate the URLs on the client in order to (among other things) avoid requiring an unnecessary barrier in the UX. Did I misunderstand?

Well we discussed that in Paris with Romain and it appears that you can't generate the URL on client side because you need to activate them server side in order to link them to the right user. We don't want to display inactives URLs (if the client generates one while being offline for example) This could leaves to strange behavior on client side.

platform done today - will need server side association capabilities to user. there is no easy way in UX to say "this is who this is". click on cog gear to change name.

(In reply to Rémy Hubscher (:natim) from comment #3) > Well we discussed that in Paris with Romain and it appears that you can't > generate the URL on client side because you need to activate them server > side in order to link them to the right user. > > We don't want to display inactives URLs (if the client generates one while > being offline for example) > This could leaves to strange behavior on client side. My understanding of the result of the Toronto discussions was that we'd need to rejigger what the URLs represent in order to make them practically generable on the client side. I suspect abr may already have theories in mind about ways forward here...

We've had this discussion with abr already also, outcome is that we want to have an url generated on the server side and given to the client so that it can change metadata associated with it. e.g. 1. Get the URL from the server 2. Let the user tweak it 3. Send the tweaks to the server so that it can store it.

Ah, gotcha. I misunderstood. Thanks!

Darrin, a few questions: - What happens to the current DnD checkbox that we have? I don't see anywhere on the mock-ups for this. - Presumably the url is displayed as soon as the panel is opened? - When does the url get changed? -- On panel open? -- When the share/copy buttons are pressed? - Can we select the url manually in the box, or is it read-only? - What's the flow for saving the invitation name when it is changed? -- i.e. do we update it when exiting the field, or only when copy/share are pressed?

(In reply to Mark Banner (:standard8) from comment #9) > Darrin, a few questions: > > - What happens to the current DnD checkbox that we have? I don't see > anywhere on the mock-ups for this. If you look at the mock-ups there is a green dot. If you click it you can enable DND. "Contacts only" does not apply to account-less and should be removed from the account-lsee UI drop down option. > - Presumably the url is displayed as soon as the panel is opened? Yes > - When does the url get changed? > -- On panel open? > -- When the share/copy buttons are pressed? It does not change, please see my comment above: * The URL has to be displayed as the user opens the panel - it is assumed that the call to get the URL from the server would be done before opening the panel for performance reasons and that the call parameters (expiration date, link generator name and invitation name) are sent to the server when the URL is copied (copy button or selection of the URL text in the text box)or shared. These parameters are stored on the server. > - Can we select the url manually in the box, or is it read-only? Yes, we should have an event on Ctrl+C or "mouse left click and select copy" so the data gets sent to the server at that moment > - What's the flow for saving the invitation name when it is changed? Described above > -- i.e. do we update it when exiting the field, or only when copy/share are > pressed? Only when copied/shared as per above

(In reply to Romain Testard [:RT] from comment #11) > > - When does the url get changed? > > -- On panel open? > > -- When the share/copy buttons are pressed? > It does not change, please see my comment above: > * The URL has to be displayed as the user opens the panel - it is assumed > that the call to get the URL from the server would be done before opening > the panel for performance reasons and that the call parameters (expiration > date, link generator name and invitation name) are sent to the server when > the URL is copied (copy button or selection of the URL text in the text > box)or shared. These parameters are stored on the server. Sorry, but this doesn't make sense to me. Firstly, as described, we'd only get a different url on each session startup. It would never be refreshed / changed so I wouldn't be able to send different urls to different places. I would have assumed that we'd get the url, and then once copied/shared we'd get a fresh url to serve. Is that what you mean? Secondly, I'm also concerned about getting a url before opening the panel. This means that regardless of if the user is going to use loop or not, we're going to have to contact the server for a url. Whilst we can do that, it seems unnecessary extra load on the server for all our FF users to do this each time on startup. I would recommend we don't do that at the moment - we stick with only get the url when we open the panel, and assess how the performance is with our testers. There's various ways we might be able to speed up the current url process without pre-fetching, but these sort of performance optimisations should first be characterised, and then dealt with separately if they are an issue.

(In reply to Mark Banner (:standard8) from comment #12) > Sorry, but this doesn't make sense to me. > > Firstly, as described, we'd only get a different url on each session > startup. It would never be refreshed / changed so I wouldn't be able to send > different urls to different places. The requirement is to have the URL displayed as you open the panel (the user should not have to press an extra button to get the URL). Once a URL is shared or copied a fresh URL could be fetched for the next time the user wants to use Loop? Or maybe when the panel is opened we could fetch that URL as we bring the panel down although this would require UX to inform the user that the URL is being fetched. Darrin what do you think? > > I would have assumed that we'd get the url, and then once copied/shared we'd > get a fresh url to serve. Is that what you mean? > > Secondly, I'm also concerned about getting a url before opening the panel. > This means that regardless of if the user is going to use loop or not, we're > going to have to contact the server for a url. > Is this significant server load Alexis? > Whilst we can do that, it seems unnecessary extra load on the server for all > our FF users to do this each time on startup. > > I would recommend we don't do that at the moment - we stick with only get > the url when we open the panel, and assess how the performance is with our > testers. There's various ways we might be able to speed up the current url > process without pre-fetching, but these sort of performance optimizations > should first be characterized, and then dealt with separately if they are an > issue.

If we can avoid hitting the server then we should do it. Pinging the server each time a browser starts is a significant number of requests, yes. Note that there are is no way to have a valid call-url if you don't ask it to the server. We could have a timer or something that shows up to the user when we're fetching the url (e.g. when the panels open. This operation is not time consuming and I believe should be acceptable). Other solution is to have the client generate the URL and then post it to the server. If we do that, then it means it's possible to have urls that *looks* valid on the client but which haven't been sent to the server yet. Maybe in this case we could grey-out the "copy" button until the URL had been sent.

(In reply to Alexis Metaireau (:alexis) from comment #14) > If we can avoid hitting the server then we should do it. > Pinging the server each time a browser starts is a significant number of > requests, yes. > It would not be each time the browser starts but rather the first time the browser starts and then each time a new URL is shared (so that you can have a URL to share thereafter) > Note that there are is no way to have a valid call-url if you don't ask it > to the server. > We could have a timer or something that shows up to the user when we're > fetching the url (e.g. when the panels open. This operation is not time > consuming and I believe should be acceptable). Sure it could be done and then cached until the user decides to share it so that we don't have a request to the server initiated each time the panel gets opened. Awaiting Darrin's feedback on a UX to be implemented when fetching a URL. > > Other solution is to have the client generate the URL and then post it to > the server. If we do that, then it means it's possible to have urls that > *looks* valid on the client but which haven't been sent to the server yet. > Maybe in this case we could grey-out the "copy" button until the URL had > been sent. I thought we discussed client side generation and agreed it was better design to rely on server side URL generation?

Oh, I understand what you mean. URLs have an expiration time that's set on the server as soon as they're defined, so you'll also need to take this into account in case you want to go this way. "Each time a browser starts" is *a lot* more than "when someone opens the panel". I believe we have technical reasons not to want a call each time a browser starts. If we don't have a really compelling reason to do that, I propose we stick with fetching the URL upon panel opening.

(In reply to Alexis Metaireau (:alexis) from comment #16) > Oh, I understand what you mean. URLs have an expiration time that's set on > the server as soon as they're defined, so you'll also need to take this into > account in case you want to go this way. Are you aware that the users will be able to set the expiration date on the client? Can't the server re-assign an expiration date when the URL is shared and the expiration data is sent to him? > > "Each time a browser starts" is *a lot* more than "when someone opens the > panel". I believe we have technical reasons not to want a call each time a > browser starts. If we don't have a really compelling reason to do that, I > propose we stick with fetching the URL upon panel opening. I wrote "It would not be each time the browser starts but rather the first time the browser starts and then each time a new URL is shared (so that you can have a URL to share thereafter)" So this is not each time the browser starts. It would be first time the browser starts and then each time a URL is shared.

(In reply to Romain Testard [:RT] from comment #17) > Can't the server re-assign an expiration date when the URL is shared and the > expiration data is sent to him? Yes the server will be able to do that. The client UI will need to be carefully crafted so that we're sure the server had the request before being able to share it. > So this is not each time the browser starts. It would be first time the > browser starts and then each time a URL is shared. Do we have numbers so I know how many requests per seconds that represents?

(In reply to Alexis Metaireau (:alexis) from comment #18) > > So this is not each time the browser starts. It would be first time the > > browser starts and then each time a URL is shared. > > Do we have numbers so I know how many requests per seconds that represents? There would be a huge spike when we ship, and if we did anything that required a regeneration of urls. As I said before, I really don't think we should do this pre-optimisation unless we have clear data that the performance is bad. I don't think there would be privacy issues, but we should check in with those folks as well before we re-architecture for this.

Prefetching the URLs on browser start is going to accrete a lot of unnecessary state in the server: 450 million URLs * the number of times an average user starts a browser per month is... a lot. I understand the desire to prevent unnecessary action on the part of the user, and that's a good thing. But the UX difference between prefetching the URL and fetching on panel open is on the order of 1/4 of a second, which is barely perceptable. This may increase slightly under load, but if we can't make an ordinary "page fetch" interval -- which is well with user's tolerance for something to happen -- then we'll need to re-think a few things anyway. When we were discussing this in Toronto, I thought we would be doing something roughly of this form: - When the panel opens, we check to see whether an "unused" URL is cached. - If so, we (a) present it to the user; and (b) send off a request to update expiration of the URL via |POST /call-url/{token}|. - If not, we get a new token from the server and, when it returns, render it to the user. This URL is written to disk so that it survives browser restarts. - When the user takes any action that could exfiltrate the URL, we mark it as "used" (this is a client designation, not something shared with the server). This ensures that a new URL is generated the next time the panel opens. These actions include: - Clicking on any of the API that we use to send via email or SMS - Copying the URL to the clipboard -- if we can't reliably detect this, then we can use text-box focus as a proxy for that action. This prevents unnecessary URL generation while providing the UX as designed.

(In reply to Romain Testard [:RT] from comment #1) > Notes after UX desktop and mobile Loop meetings: > > * The URL should be short enough to be easily copied through text selection > or to be share by SMS. This means a maximum of 8 characters to identify the > call as part of the full URL It's not a big change, but this is going to need to be something more on the order of 11 characters. See my analysis here: https://wiki.mozilla.org/Loop/Architecture/MVP#Token_Length_and_Generation With only 8 characters, the token space is insufficiently sparse; unsolicited nuisance calls become a real problem at this length.

Handling optimisation of URL load is clearly extra work on top of what was originally intended in this bug. Therefore I've split it out to bug 1030961. For this bug we'll stick to requesting a new url each time the panel opens. This way we can also assess it before working out when to schedule the extra rework that bug 1030961 would take.

(In reply to Adam Roach [:abr] from comment #21) > (In reply to Romain Testard [:RT] from comment #1) > > Notes after UX desktop and mobile Loop meetings: > > > > * The URL should be short enough to be easily copied through text selection > > or to be share by SMS. This means a maximum of 8 characters to identify the > > call as part of the full URL > > It's not a big change, but this is going to need to be something more on the > order of 11 characters. See my analysis here: > https://wiki.mozilla.org/Loop/Architecture/MVP#Token_Length_and_Generation > > With only 8 characters, the token space is insufficiently sparse; > unsolicited nuisance calls become a real problem at this length. Agreed and thanks for the analysis

Screenshot of the new simplified version of the UI. The rest of the components will be implemented separately

Comment on attachment 8449564 [details] Screen Shot 2014-07-02 at 10.23.48 AM.png This is a great step in the right direction. Where did the string "Share the link below..." come from? Also note that a visual spec exists in this bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1016087) where you can reference padding, text colors, border radii, etc. (specifically http://people.mozilla.org/~mmaslaney/loop/Loop-spec.png)

Comment on attachment 8449563 [details] Screen Shot 2014-07-02 at 10.23.38 AM.png Same comments as other screenshot -- please look at text colors and border radii for the heading and text input. Also -- I believe the spec at https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#precall calls the 'Unavailable' mode 'Do Not Disturb'.

(In reply to Darrin Henein [:darrin] from comment #28) > Comment on attachment 8449563 [details] > Screen Shot 2014-07-02 at 10.23.38 AM.png > > Same comments as other screenshot -- please look at text colors and border > radii for the heading and text input. > > Also -- I believe the spec at > https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#precall calls the > 'Unavailable' mode 'Do Not Disturb'. Thanks. I'll make the appropriate changes.

Comment on attachment 8448881 [details] [diff] [review] Implement new UX for loop panel Waiting for updated patch to review.

Took all the values for padding margins etc from the mockups. The popup menu does get cut off but that bug is already being handled so the problem should resolve itself.

Comment on attachment 8450311 [details] Screen Shot 2014-07-03 at 9.39.36 AM.png This looks better, thanks! Can you add one screenshot showing the hover state on the availability indicator/label?

I hope this is the one you meant :)

Comment on attachment 8452873 [details] [diff] [review] Implement new UX for loop panel Review of attachment 8452873 [details] [diff] [review]: ----------------------------------------------------------------- Looks generally good. Unfinished review, we'll switch to pair-reviewing it with :demose and :andreio over video, but this should be a good starting point :) ::: browser/components/loop/content/js/panel.jsx @@ +40,5 @@ > + this.setState({showMenu: false}); > + }, > + > + changeAvailability: function(event) { > + var status = event.target.getAttribute('data-status'); nit: I think we went with using double-quotes for simple strings everywhere else in the Loop code. @@ +41,5 @@ > + }, > + > + changeAvailability: function(event) { > + var status = event.target.getAttribute('data-status'); > + switch(status) { nit: space before opening parenthesis. @@ +48,5 @@ > + navigator.mozLoop.doNotDisturb = false; > + break; > + case 'donotdisturb': > + this.setState({doNotDisturb: true}); > + navigator.mozLoop.doNotDisturb = true; You could safely use a single assignment for navigator.mozLoop.doNotDisturb after the switch statement, using navigator.mozLoop.doNotDisturb = this.state.doNotDisturb (and please add a comment alerting for side effect.) @@ +60,5 @@ > + var cx = React.addons.classSet; > + var availabilityStatus = cx({ > + 'status': true, > + 'status-dnd': navigator.mozLoop.doNotDisturb, > + 'status-available': !navigator.mozLoop.doNotDisturb As you have that value carried by state, it's probably safe to just use this.state.doNotDisturb here. @@ +74,5 @@ > return ( > + <div className="footer component-spacer"> > + <div className="do-not-disturb"> > + <p className="dnd-status"> > + <span onClick={this.showDropdownMenu}>{availabilityText}</span> The onClick event handler being applied for the sole text, clicking on the pill doesn't work. We should ensure that clicking anywhere in the parent element actually opens the menu. @@ +81,5 @@ > + <ul className={availabilityDropdown} > + onMouseLeave={this.hideDropdownMenu}> > + <li className="dnd-menu-item dnd-make-available"> > + <i className="status status-available"></i> > + <span onClick={this.changeAvailability} Could we use an <a/> tag here? Would seem more semantically correct to me. @@ +88,5 @@ > + </span> > + </li> > + <li className="dnd-menu-item dnd-make-unavailable"> > + <i className="status status-dnd"></i> > + <span onClick={this.changeAvailability} Same remark as above. @@ +94,5 @@ > + {__("display_name_dnd_status")} > + </span> > + </li> > + </ul> > + </div> This feels to me like something that could become a reusable agnostic component, eg. <DropdownMenu options={options} onChange={handler}/>, especially as I think I've seen the concept reused elsewhere in the mockups. Thoughts? This could be done in a followup bug, which should then be created if this land as-is. @@ +156,1 @@ > }, Please add jsdoc explaining what purpose this actually serves. @@ +184,5 @@ > return ( > <PanelLayout summary={__("get_link_to_share")}> > + <div className="invite"> > + <input type="url" value={this.state.callUrl} > + ref="caller" readOnly="true" As refs are no more used after form submission, you can safely remove the ref attribute here. @@ +248,5 @@ > * @link http://www.w3.org/TR/page-visibility/ > */ > _registerVisibilityChangeEvent: function() { > + // XXX pass in the visibility status to detect when to generate a new > + // panel view I think this is addressed in initialize() where it listens to panel:open to reset the panel. @@ +305,4 @@ > PanelView: PanelView, > PanelRouter: PanelRouter, > + ToSView: ToSView, > + CallUrlResult: CallUrlResult nit: let's try to keep exports in alphabetic order. ::: browser/components/loop/content/shared/css/common.css @@ +69,5 @@ > border-radius: .2em; > } > > .btn-info { > + background: #0095dd; Please also implement :hover (#008ACB) and :active (#006B9D) while you're there :) ::: browser/components/loop/content/shared/css/panel.css @@ +38,5 @@ > } > > +.description-content { > + margin: .5em 0; > + font-size: 11px; I'm finding text a bit small and hard to read, especially on a large screen (I'm using a 24" atm). I'd advocate for increasing the size by at least one pixel, but that might be challenged. Using media queries won't probably help because the panel window width is fixed though… @@ +53,5 @@ > .share .action input[type="url"] { > border: 1px solid #ccc; /* Overriding background style for a text input (see > below) resets its borders to a weird beveled style; > defining a default 1px border solves the issue. */ > + font-size: 11px; Same remark with the very small, hardly legible font size on larger screens. @@ +84,5 @@ > + > +/* Call URL input */ > + > +.input-controls { > + display: flex; Is this actually necessary? atm it's a simple full width text input which shouldn't require such a setup if you ask me… @@ +100,5 @@ > +.dnd-status:hover { > + border: 1px solid #DDD; > + background: #F1F1F1; > + border-radius: 3px; > + cursor: pointer; Curious: why on hover only? Also this is misleading because the whole content covered by this selector isn't actually clickable (see previous comment about this). @@ +122,5 @@ > +.dnd-menu-item { > + text-align: left; > + margin: .3em 0; > + padding: .2em .5em; > + cursor: pointer; Using a standard <a/> tag arount the matching element wouldn't require this. @@ +140,5 @@ > +.status { > + width: 8px; > + height: 8px; > + border-radius: 50%; > + display: inline-block; nit: I usually tend to put "display" and "position" rules at the top because they're so important @@ +171,5 @@ > + > +/* Footer */ > + > +.footer { > + font-size: 12px; Really, we should use 12px everywhere (see previous comments about using 11px). ::: browser/components/loop/test/desktop-local/panel_test.js @@ +180,5 @@ > > it("should toggle the value of mozLoop.doNotDisturb", function() { > + var dropdownMenu = getElementByTag(view, "ul"); > + var menuItem = getElementByClass(view, "dnd-make-available"); > + var availableMenuOption = getElementByTag(menuItem, "span"); As the test component is mounted, how about simply using view.getDOMNode().querySelector(".dnd-make-available span")?

Comment on attachment 8452474 [details] Screen Shot 2014-07-08 at 11.08.20 AM.png Looks great, thanks! (yes that was the one I meant!)

Nothing should have changed visually. I just changed the text size from px -> ems.

(In reply to Nicolas Perriault (:NiKo`) from comment #38) > Comment on attachment 8452873 [details] [diff] [review] > Implement new UX for loop panel > > Review of attachment 8452873 [details] [diff] [review]: > ----------------------------------------------------------------- > > @@ +48,5 @@ > > + navigator.mozLoop.doNotDisturb = false; > > + break; > > + case 'donotdisturb': > > + this.setState({doNotDisturb: true}); > > + navigator.mozLoop.doNotDisturb = true; > > You could safely use a single assignment for navigator.mozLoop.doNotDisturb > after the switch statement, using navigator.mozLoop.doNotDisturb = > this.state.doNotDisturb (and please add a comment alerting for side effect.) Looking at the react docs [0] setState() does not immediately mutate this.state so the new value is not available for usage in the scope of the function. http://facebook.github.io/react/docs/component-api.html#setstate > > @@ +74,5 @@ > > return ( > > + <div className="footer component-spacer"> > > + <div className="do-not-disturb"> > > + <p className="dnd-status"> > > + <span onClick={this.showDropdownMenu}>{availabilityText}</span> > > The onClick event handler being applied for the sole text, clicking on the > pill doesn't work. We should ensure that clicking anywhere in the parent > element actually opens the menu. This worked fine in this case. In the dropdown menu you have to check which element was clicked (inside the same handler). Because the element with onClick handler has children, when the handler is called it could be either it or its children. Checking for a HTML class selector on the element won't work. I used the solution from here [1] and explained in the comments what is going on. [1] http://stackoverflow.com/a/21664414 > > @@ +81,5 @@ > > + <ul className={availabilityDropdown} > > + onMouseLeave={this.hideDropdownMenu}> > > + <li className="dnd-menu-item dnd-make-available"> > > + <i className="status status-available"></i> > > + <span onClick={this.changeAvailability} > > Could we use an <a/> tag here? Would seem more semantically correct to me. > I think a <span> is better in this case because the spec [2] mentions anchor tags to have a destination which is not the case here. i.e. it's not a link that might be used as a fallback in case JS is disabled (as you might encounter in SPA). [2] http://www.w3.org/TR/html401/struct/links.html > @@ +248,5 @@ > > * @link http://www.w3.org/TR/page-visibility/ > > */ > > _registerVisibilityChangeEvent: function() { > > + // XXX pass in the visibility status to detect when to generate a new > > + // panel view > > I think this is addressed in initialize() where it listens to panel:open to > reset the panel. This is just a mentioned that the panel gets resetted every time it drops down and that it should be fixed. > > I'm finding text a bit small and hard to read, especially on a large screen > (I'm using a 24" atm). I'd advocate for increasing the size by at least one > pixel, but that might be challenged. > > Using media queries won't probably help because the panel window width is > fixed though… > I switched over to 12px with permission from Darrin, and now we're using ems :)

Comment on attachment 8453342 [details] [diff] [review] Implement new UX for loop panel Review of attachment 8453342 [details] [diff] [review]: ----------------------------------------------------------------- Looks good; r=dmose.

Comment on attachment 8453342 [details] [diff] [review] Implement new UX for loop panel Review of attachment 8453342 [details] [diff] [review]: ----------------------------------------------------------------- Testing turned up a string issue; new patch shortly.

(In reply to Andrei Oprea [:andreio] from comment #43) > > Looking at the react docs [0] setState() does not immediately mutate > this.state so the new value is not available for usage in the scope of the > function. Ok, so how about the other way around? switch(status) { case 'available': navigator.mozLoop.doNotDisturb = false; break; case 'donotdisturb': navigator.mozLoop.doNotDisturb = false; break; } this.setState({doNotDisturb: navigator.mozLoop.doNotDisturb}); Which could actually be further simplified to: navigator.mozLoop.doNotDisturb = status === "donotdisturb"; this.setState({doNotDisturb: navigator.mozLoop.doNotDisturb}); > This worked fine in this case. In the dropdown menu you have to check which > element was clicked (inside the same handler). > [1] http://stackoverflow.com/a/21664414 Another comment notes that “event.target gives you the native DOM node”, which could also help. > > Could we use an <a/> tag here? Would seem more semantically correct to me. > > > I think a <span> is better in this case because the spec [2] mentions anchor > tags to have a destination which is not the case here. I see, and how about using a <button>? You could even enclose the pill in it: <button type="button" onClick={this.showDropdownMenu}> <span>{availabilityText}</span> <i className={availabilityStatus}></i> </button> > > > + // XXX pass in the visibility status to detect when to generate a new > > > + // panel view > This is just a mentioned that the panel gets resetted every time it drops > down and that it should be fixed. Oh, I see. You should remove the "XXX" as it usually indicates something to fix. > I switched over to 12px with permission from Darrin, and now we're using ems > :) This is cool \o/ Also and because I lost my other pending review draft, please don't forget to update the translation strings as discussed during our call :)

Niko and I chatted; because it's important to try and get people working on the panel unblocked by the European morning (which is not far off now), I'm going to go ahead and land this with r=dmose, ui-r=darrin. Darrin has given ui-r to almost-but-not-quite-exactly-identical patch (we switched some font size stuff to use ems instead of px), and I'm going to assume that's good enough. Darrin, if that's wrong, I apologize, and work with you to sort out any resulting issues tomorrow.

https://hg.mozilla.org/mozilla-central/rev/90d62dd3709a https://hg.mozilla.org/mozilla-central/rev/bad360d2339f

Comment on attachment 8453314 [details] review.jpg Awesome.

Marking this verified based on prior smoketesting.