Add button to Pocket doorhanger to request mobile app

RESOLVED FIXED in Firefox 62

Status

()

--
enhancement
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: anthony, Assigned: anthony)

Tracking

unspecified
Firefox 62
Points:
---

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

a year ago
By showing users an upsell, positioned as a feature, as part of the save confirmation and make it easy for users to get the mobile app, more users will understand the value of the Pocket mobile and start using the mobile app.

The initial experiment will target existing logged in users only.

We've added flags to the server response on the save request that:
- Set whether the user has the mobile app
- Which variant of the experiment the user should be enrolled in

If the user has an account, but they've never installed the Android or iOS app, we will display a new button inline in the current panel that will email a link to the mobile application to the user.

Here's the PR:
https://github.com/mozilla-partners/pocket/pull/21
Posted file Github PR
We're trying to work on the the pocket commit process.

Would you prefer to look at the Github PR?

Or should we create a pull on MozReview?

Or should someone else review now?
Attachment #8965421 - Flags: review?(gijskruitbosch+bugs)

Comment 2

a year ago
Comment on attachment 8965421 [details] [review]
Github PR

(In reply to Mike Kaply [:mkaply] from comment #1)
> Created attachment 8965421 [details] [review]
> Github PR
> 
> We're trying to work on the the pocket commit process.
> 
> Would you prefer to look at the Github PR?
> 
> Or should we create a pull on MozReview?
> 
> Or should someone else review now?

I think given that it's merged to github already, it'd make more sense as a mozreview request in that it's odd to put comments on github for a merged PR, also when the review request is on bugzilla. The downside there is that then any changes need mirroring back to github somehow. It's kind of a lose-lose situation. For shield, we did reviews in github prior to merging, but I think the team wasn't super happy with that workflow either, and now the normandy code has just moved into m-c properly. It'd sure be simpler to do that with pocket, too, but I dunno if that fits with the plans Pocket have.

In any case, Jared or I (or other Firefox peers, really) could review m-c changes.

For now, I think I have a number of general-ish comments:

1) it looks like none of the new work is localized. That's not OK for stuff landing on m-c - we can't send English content to everyone and hope for the best. Is the experiment limited to people using the en-US UI locale? I don't see that in the code off-hand, but perhaps I'm missing where that's happening?

2) there's a bunch of strange stuff in the CSS that's changing, e.g.:

+.pkt_ext_containersaved
 .pkt_ext_containersaved .pkt_ext_recenttag_detail h4,
 .pkt_ext_containersaved .pkt_ext_suggestedtag_detail h4 {

This is a strange selector, because it's really:


.pkt_ext_containersaved .pkt_ext_containersaved .pkt_ext_recenttag_detail h4,
.pkt_ext_containersaved .pkt_ext_suggestedtag_detail h4 {

I assume there should be a comma there? The body of this rule is also weird:

     font-size: 0.8125em;
     font-size: 13px;

Not sure what's going on there. Also not sure how related that is to this change and if we should maybe have a separate commit for dealing with some formatting in the CSS + getting rid of firasans (which is happening here, but I think is happening because of https://bugzilla.mozilla.org/show_bug.cgi?id=1385472 ).

3)  content/panels/img/pocketmenuitem16@2x.png and its non-hidpi equivalent are changing in this PR, but I don't really understand why/how.

4) I'd *really* like to bring Pocket in line with the style guides we use for the rest of m-c, e.g. for trailing commas on object property definitions, where operators go, etc. Maybe not as part of this PR though...
5) A lot of the xhr-related code I'd like to see using async/await instead of nested callbacks.
Attachment #8965421 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 3

a year ago
Hi Gijs, thanks for the feedback! I'll respond to each item individually:

1: We get the locale from the prefs, and send that in all headers to our backend. We're using this on the save request to determine the flags sent back to the client. It's these flags that we use to determine whether to show the experiment. I do understand how this could be worrisome, so I'd be happy to add an extra check for language.

2: I'll fix the CSS selectors. I removed Firasans as I noticed it wasn't loading anyways and was throwing errors. In hindsight I agree it should have been a separate PR. 

3. These images are new to the repository, but do not need to go live at this time. I'll remove them.

4. & 5. We'd really like to get in line with your style guides as well. For these changes I matched the style as it currently exists for consistency sake. That being said, we plan to bring this version of the extension to parity with our open-sourced version, and it would be great to make sure that one meets your requirements before we begin that project.

I'll get started on the above changes right away. Given the unknowns in regards to process, how would you like me to submit these? I'd be happy to set up a meeting sometime to go over this if it would help. Thanks!

Comment 4

a year ago
(In reply to Anthony from comment #3)
> I'll get started on the above changes right away. Given the unknowns in
> regards to process, how would you like me to submit these? I'd be happy to
> set up a meeting sometime to go over this if it would help. Thanks!

I guess at this point a mozreview request would be easiest, and you may want to then port any changes back from m-c to the github repo once it merges. https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html has reasonable documentation on how to submit commits via mozreview. Feel free to ping me on here or email, or ask around in #fx-team or #developers on IRC, if you get stuck with how to use it - it can take a bit of getting used to.
Comment hidden (mozreview-request)

Comment 6

10 months ago
mozreview-review
Comment on attachment 8975531 [details]
Bug 1451840 - Add button to Pocket doorhanger to request mobile app

https://reviewboard.mozilla.org/r/243786/#review249944

OK, several bits of feedback here - not really that much considering the size of the patch... mainly to do with making sure the panels are usable with the keyboard and with accessbility tools, as well as some issues with the image cache thingy, RTL, and otherwise mostly code style nits. If it's easier for your workflow, I can also review modifications you make to address these comments in github. Thanks!

::: browser/extensions/pocket/content/main.js:262
(Diff revision 1)
>  
> -                // Add url
> +                    // Add url
> -                var options = {
> +                    var options = {
> -                    success(data, request) {
> +                        success(data, request) {
> -                        var item = data.item;
> +                            var item = data.item;
> +                            var ho2 = data.ho2;

Out of interest, what does `ho2` stand for?

::: browser/extensions/pocket/content/panels/css/saved.css:830
(Diff revision 1)
> +
> +/*=Coral Button
> +--------------------------------------------------------------------------------------- */
> +.pkt_ext_button {
> +    padding: 3px;
> +    background-color: #EF4056;

It's good practice to specify the color as well, otherwise the result might be unreadable on systems where the default foreground colour is different.

::: browser/extensions/pocket/content/panels/css/saved.css:846
(Diff revision 1)
> +.pkt_ext_button:after {
> +    clear: both;
> +    content: '';
> +    display: table;
> +}

What's this? A `clear` hack? But I don't see any floats... why do you need this? Especially the `display: table` on a pseudoelement is a bit odd...

::: browser/extensions/pocket/content/panels/css/saved.css:854
(Diff revision 1)
> +    display: table;
> +}
> +
> +/* alt button */
> +.pkt_ext_blue_button {
> +    background-color: #0060df;

Here too we should probably specify the foreground colour.

::: browser/extensions/pocket/content/panels/css/saved.css:867
(Diff revision 1)
> +    height: 15px;
> +    width: 15px;

The image is 44x44px, so this resize doesn't seem great in terms of achieving something that looks nice on a non-hidpi screen (and might even be blurry on some hidpi ones). Can we source a 32x32px image and size it to 16px, maybe?

::: browser/extensions/pocket/content/panels/css/saved.css:890
(Diff revision 1)
> +
> +.pkt_ext_saved_tmplogin {
> +    height: 146px;
> +}
> +
> +.pkt_ext_indent_border_left {

This isn't RTL-friendly and should probably be using logical-start margins/paddings/borders instead (and possibly switch the name to `border_start` or something like that) to avoid looking strange in RTL locales like Hebrew and Arabic.

::: browser/extensions/pocket/content/panels/css/saved.css:895
(Diff revision 1)
> +.pkt_ext_indent_border_left {
> +    margin: 15px 0;
> +    padding-left: 7px;
> +    border-left: 4px solid rgba(68, 68, 68, .15);
> +    font-size: 13px;
> +    text-align: left;

I don't think you need to specify this? :-)

::: browser/extensions/pocket/content/panels/css/saved.css:899
(Diff revision 1)
> +    font-size: 13px;
> +    text-align: left;
> +    color: #313131;
> +}
> +
> +.pkt_ext_indent_border_left p {

Nit: could use a child selector here, as there are no non-child descendant `p`s anyway.

::: browser/extensions/pocket/content/panels/css/sendtomobile.css:2
(Diff revision 1)
> +.pkt_ext_containersaved .pkt_ext_saved_sendtomobile {
> +    z-index: 1;

This is the only use of z-index I'm seeing - why do you need this?

::: browser/extensions/pocket/content/panels/css/sendtomobile.css:25
(Diff revision 1)
> +    position: absolute;
> +    height: 15px;
> +    width: 10px;
> +    top: 0;
> +    left: -16px;
> +    content: "";

If this is really an image, you could just put the `url()` directly in the `content` property.

::: browser/extensions/pocket/content/panels/css/sendtomobile.css:26
(Diff revision 1)
> +    height: 15px;
> +    width: 10px;
> +    top: 0;
> +    left: -16px;
> +    content: "";
> +    background-image: url('data:image/svg+xml;utf8,<svg viewBox="0 0 10 15" xmlns="http://www.w3.org/2000/svg"><g stroke="none" stroke-width="1" fill="none" fill-rule="evenodd"><g transform="translate(-714.000000, -291.000000)" fill="white"><g transform="translate(635.000000, 140.000000)"><g transform="translate(16.000000, 142.000000)"><g transform="translate(63.000000, 9.000000)"><path d="M0.81,0.407552083 L9.19,0.407552083 C9.63735065,0.407552083 10,0.770201436 10,1.21755208 L10,14.0233333 C10,14.470684 9.63735065,14.8333333 9.19,14.8333333 L0.81,14.8333333 C0.362649353,14.8333333 6.09896166e-16,14.470684 5.55111512e-16,14.0233333 L2.22044605e-16,1.21755208 C1.67259951e-16,0.770201436 0.362649353,0.407552083 0.81,0.407552083 Z M1,1.31865406 L1,12.074719 L9,12.074719 L9,1.31865406 L1,1.31865406 Z M3.68055556,13.1666667 C3.48879002,13.1666667 3.33333333,13.3221234 3.33333333,13.5138889 C3.33333333,13.7056544 3.48879002,13.8611111 3.68055556,13.8611111 L6.31944444,13.8611111 C6.51120998,13.8611111 6.66666667,13.7056544 6.66666667,13.5138889 C6.66666667,13.3221234 6.51120998,13.1666667 6.31944444,13.1666667 L3.68055556,13.1666667 Z"></path></g></g></g></g></g></svg>');

This SVG too looks like it wants simplifying - I count 4 different translations (and, as far as I can tell, just to get it back to the origin...) and also a redundant `fill` attribute.

::: browser/extensions/pocket/content/panels/css/sendtomobile.css:31
(Diff revision 1)
> +    background-image: url('data:image/svg+xml;utf8,<svg viewBox="0 0 10 15" xmlns="http://www.w3.org/2000/svg"><g stroke="none" stroke-width="1" fill="none" fill-rule="evenodd"><g transform="translate(-714.000000, -291.000000)" fill="white"><g transform="translate(635.000000, 140.000000)"><g transform="translate(16.000000, 142.000000)"><g transform="translate(63.000000, 9.000000)"><path d="M0.81,0.407552083 L9.19,0.407552083 C9.63735065,0.407552083 10,0.770201436 10,1.21755208 L10,14.0233333 C10,14.470684 9.63735065,14.8333333 9.19,14.8333333 L0.81,14.8333333 C0.362649353,14.8333333 6.09896166e-16,14.470684 5.55111512e-16,14.0233333 L2.22044605e-16,1.21755208 C1.67259951e-16,0.770201436 0.362649353,0.407552083 0.81,0.407552083 Z M1,1.31865406 L1,12.074719 L9,12.074719 L9,1.31865406 L1,1.31865406 Z M3.68055556,13.1666667 C3.48879002,13.1666667 3.33333333,13.3221234 3.33333333,13.5138889 C3.33333333,13.7056544 3.48879002,13.8611111 3.68055556,13.8611111 L6.31944444,13.8611111 C6.51120998,13.8611111 6.66666667,13.7056544 6.66666667,13.5138889 C6.66666667,13.3221234 6.51120998,13.1666667 6.31944444,13.1666667 L3.68055556,13.1666667 Z"></path></g></g></g></g></g></svg>');
> +    background-repeat: no-repeat;
> +}
> +
> +.pkt_ext_logo_action_copy {
> +    color: white;

It would be easier to understand/modify the CSS here if the color was set together with the background color (which is set several elements higher up).

::: browser/extensions/pocket/content/panels/css/sendtomobile.css:78
(Diff revision 1)
> +    margin: 0 auto;
> +    line-height: 20px;
> +}
> +
> +.pkt_ext_bold {
> +    color: #666;

You don't need to re-specify the color here, AFAICT.

::: browser/extensions/pocket/content/panels/css/sendtomobile.css:212
(Diff revision 1)
> +    background: linear-gradient(to right, #EEE 0%, #DDD 50%, #EEE 100%);
> +    background-size: 400% 400%;
> +    animation: backgroundScroll 3s linear infinite;

Looks like this could be unified with th eprevious selector. But also, could you just animate the colour directly instead of the bg scroll you're using here to make the background colour pulse?

::: browser/extensions/pocket/content/panels/img/app_store_dowload_apple.svg:1
(Diff revision 1)
> +<svg xmlns="http://www.w3.org/2000/svg" width="119.664" height="40" viewBox="0 0 119.66407 40"><path d="M110.135 0H9.535c-.367 0-.73 0-1.095.002-.306.002-.61.008-.919.013A13.215 13.215 0 0 0 5.517.19a6.665 6.665 0 0 0-1.9.627 6.438 6.438 0 0 0-1.62 1.18A6.258 6.258 0 0 0 .82 3.617a6.601 6.601 0 0 0-.625 1.903 12.993 12.993 0 0 0-.179 2.002c-.01.307-.01.615-.015.921V31.56c.005.31.006.61.015.921a12.992 12.992 0 0 0 .18 2.002 6.588 6.588 0 0 0 .624 1.905A6.208 6.208 0 0 0 1.998 38a6.274 6.274 0 0 0 1.618 1.179 6.7 6.7 0 0 0 1.901.63 13.455 13.455 0 0 0 2.004.177c.31.007.613.011.919.011.366.002.728.002 1.095.002h100.6c.36 0 .724 0 1.084-.002.304 0 .617-.004.922-.01a13.279 13.279 0 0 0 2-.178 6.804 6.804 0 0 0 1.908-.63A6.277 6.277 0 0 0 117.666 38a6.395 6.395 0 0 0 1.182-1.614 6.604 6.604 0 0 0 .619-1.905 13.506 13.506 0 0 0 .185-2.002c.004-.31.004-.61.004-.921.008-.364.008-.725.008-1.094V9.536c0-.366 0-.73-.008-1.092 0-.306 0-.614-.004-.92a13.507 13.507 0 0 0-.185-2.003 6.618 6.618 0 0 0-.62-1.903 6.466 6.466 0 0 0-2.798-2.8 6.768 6.768 0 0 0-1.908-.627 13.044 13.044 0 0 0-2-.176c-.305-.005-.618-.011-.922-.013-.36-.002-.725-.002-1.084-.002z" fill="#a6a6a6"/><path d="M8.445 39.125c-.305 0-.602-.004-.904-.01a12.687 12.687 0 0 1-1.87-.164 5.884 5.884 0 0 1-1.656-.548 5.406 5.406 0 0 1-1.397-1.016 5.32 5.32 0 0 1-1.02-1.397 5.722 5.722 0 0 1-.544-1.657 12.414 12.414 0 0 1-.166-1.875c-.007-.21-.015-.913-.015-.913v-23.1s.009-.692.015-.895a12.37 12.37 0 0 1 .165-1.872 5.755 5.755 0 0 1 .544-1.662 5.373 5.373 0 0 1 1.015-1.398 5.565 5.565 0 0 1 1.402-1.023 5.823 5.823 0 0 1 1.653-.544A12.586 12.586 0 0 1 7.543.887l.902-.012h102.769l.913.013a12.385 12.385 0 0 1 1.858.162 5.938 5.938 0 0 1 1.671.548 5.594 5.594 0 0 1 2.415 2.42 5.763 5.763 0 0 1 .535 1.649 12.995 12.995 0 0 1 .174 1.887c.003.283.003.588.003.89.008.375.008.732.008 1.092v20.929c0 .363 0 .718-.008 1.075 0 .325 0 .623-.004.93a12.731 12.731 0 0 1-.17 1.853 5.739 5.739 0 0 1-.54 1.67 5.48 5.48 0 0 1-1.016 1.386 5.413 5.413 0 0 1-1.4 1.022 5.862 5.862 0 0 1-1.668.55 12.542 12.542 0 0 1-1.869.163c-.293.007-.6.011-.897.011l-1.084.002z"/><g data-name="&lt;Group&gt;"><g data-name="&lt;Group&gt;" fill="#fff"><path data-name="&lt;Path&gt;" d="M24.769 20.3a4.949 4.949 0 0 1 2.356-4.151 5.066 5.066 0 0 0-3.99-2.158c-1.68-.176-3.308 1.005-4.164 1.005-.872 0-2.19-.988-3.608-.958a5.315 5.315 0 0 0-4.473 2.728c-1.934 3.348-.491 8.269 1.361 10.976.927 1.325 2.01 2.805 3.428 2.753 1.387-.058 1.905-.885 3.58-.885 1.658 0 2.144.885 3.59.852 1.489-.025 2.426-1.332 3.32-2.67a10.962 10.962 0 0 0 1.52-3.092 4.782 4.782 0 0 1-2.92-4.4zM22.037 12.21a4.872 4.872 0 0 0 1.115-3.49 4.957 4.957 0 0 0-3.208 1.66A4.636 4.636 0 0 0 18.8 13.74a4.1 4.1 0 0 0 3.237-1.53z"/></g><g fill="#fff"><path d="M42.302 27.14H37.57l-1.137 3.356h-2.005l4.484-12.418h2.083l4.483 12.418h-2.039zm-4.243-1.55h3.752l-1.85-5.446h-.051zM55.16 25.97c0 2.813-1.506 4.62-3.779 4.62a3.07 3.07 0 0 1-2.848-1.583h-.043v4.484H46.63V21.442h1.8v1.506h.033a3.212 3.212 0 0 1 2.883-1.6c2.298 0 3.813 1.816 3.813 4.622zm-1.91 0c0-1.833-.948-3.038-2.393-3.038-1.42 0-2.375 1.23-2.375 3.038 0 1.824.955 3.046 2.375 3.046 1.445 0 2.393-1.197 2.393-3.046zM65.125 25.97c0 2.813-1.506 4.62-3.779 4.62a3.07 3.07 0 0 1-2.848-1.583h-.043v4.484h-1.859V21.442h1.799v1.506h.034a3.212 3.212 0 0 1 2.883-1.6c2.298 0 3.813 1.816 3.813 4.622zm-1.91 0c0-1.833-.948-3.038-2.393-3.038-1.42 0-2.375 1.23-2.375 3.038 0 1.824.955 3.046 2.375 3.046 1.445 0 2.392-1.197 2.392-3.046zM71.71 27.036c.138 1.232 1.334 2.04 2.97 2.04 1.566 0 2.693-.808 2.693-1.919 0-.964-.68-1.54-2.29-1.936l-1.609-.388c-2.28-.55-3.339-1.617-3.339-3.348 0-2.142 1.867-3.614 4.519-3.614 2.624 0 4.423 1.472 4.483 3.614h-1.876c-.112-1.239-1.136-1.987-2.634-1.987s-2.521.757-2.521 1.858c0 .878.654 1.395 2.255 1.79l1.368.336c2.548.603 3.606 1.626 3.606 3.443 0 2.323-1.85 3.778-4.793 3.778-2.754 0-4.614-1.42-4.734-3.667zM83.346 19.3v2.142h1.722v1.472h-1.722v4.991c0 .776.345 1.137 1.102 1.137a5.808 5.808 0 0 0 .611-.043v1.463a5.104 5.104 0 0 1-1.032.086c-1.833 0-2.548-.689-2.548-2.445v-5.189h-1.316v-1.472h1.316V19.3zM86.065 25.97c0-2.849 1.678-4.639 4.294-4.639 2.625 0 4.295 1.79 4.295 4.639 0 2.856-1.661 4.638-4.295 4.638-2.633 0-4.294-1.782-4.294-4.638zm6.695 0c0-1.954-.895-3.108-2.401-3.108s-2.4 1.162-2.4 3.108c0 1.962.894 3.106 2.4 3.106s2.401-1.144 2.401-3.106zM96.186 21.442h1.773v1.541h.043a2.16 2.16 0 0 1 2.177-1.635 2.866 2.866 0 0 1 .637.069v1.738a2.598 2.598 0 0 0-.835-.112 1.873 1.873 0 0 0-1.937 2.083v5.37h-1.858zM109.384 27.837c-.25 1.643-1.85 2.771-3.898 2.771-2.634 0-4.269-1.764-4.269-4.595 0-2.84 1.644-4.682 4.19-4.682 2.506 0 4.08 1.72 4.08 4.466v.637h-6.394v.112a2.358 2.358 0 0 0 2.436 2.564 2.048 2.048 0 0 0 2.09-1.273zm-6.282-2.702h4.526a2.177 2.177 0 0 0-2.22-2.298 2.292 2.292 0 0 0-2.306 2.298z"/></g></g><g data-name="&lt;Group&gt;"><g fill="#fff"><path d="M37.826 8.731a2.64 2.64 0 0 1 2.808 2.965c0 1.906-1.03 3.002-2.808 3.002h-2.155V8.73zm-1.228 5.123h1.125a1.876 1.876 0 0 0 1.967-2.146 1.881 1.881 0 0 0-1.967-2.134h-1.125zM41.68 12.444a2.133 2.133 0 1 1 4.248 0 2.134 2.134 0 1 1-4.247 0zm3.334 0c0-.976-.439-1.547-1.208-1.547-.773 0-1.207.571-1.207 1.547 0 .984.434 1.55 1.207 1.55.77 0 1.208-.57 1.208-1.55zM51.573 14.698h-.922l-.93-3.317h-.07l-.927 3.317h-.913l-1.242-4.503h.902l.806 3.436h.067l.926-3.436h.852l.926 3.436h.07l.803-3.436h.889zM53.854 10.195h.855v.715h.066a1.348 1.348 0 0 1 1.344-.802 1.465 1.465 0 0 1 1.559 1.675v2.915h-.889v-2.692c0-.724-.314-1.084-.972-1.084a1.033 1.033 0 0 0-1.075 1.141v2.635h-.888zM59.094 8.437h.888v6.26h-.888zM61.218 12.444a2.133 2.133 0 1 1 4.247 0 2.134 2.134 0 1 1-4.247 0zm3.333 0c0-.976-.439-1.547-1.208-1.547-.773 0-1.207.571-1.207 1.547 0 .984.434 1.55 1.207 1.55.77 0 1.208-.57 1.208-1.55zM66.4 13.424c0-.81.604-1.278 1.676-1.344l1.22-.07v-.389c0-.475-.315-.744-.922-.744-.497 0-.84.182-.939.5h-.86c.09-.773.818-1.27 1.84-1.27 1.128 0 1.765.563 1.765 1.514v3.077h-.855v-.633h-.07a1.515 1.515 0 0 1-1.353.707 1.36 1.36 0 0 1-1.501-1.348zm2.895-.384v-.377l-1.1.07c-.62.042-.9.253-.9.65 0 .405.351.64.834.64a1.062 1.062 0 0 0 1.166-.983zM71.348 12.444c0-1.423.732-2.324 1.87-2.324a1.484 1.484 0 0 1 1.38.79h.067V8.437h.888v6.26h-.851v-.71h-.07a1.563 1.563 0 0 1-1.415.785c-1.145 0-1.869-.901-1.869-2.328zm.918 0c0 .955.45 1.53 1.203 1.53.75 0 1.212-.583 1.212-1.526 0-.938-.468-1.53-1.212-1.53-.748 0-1.203.58-1.203 1.526zM79.23 12.444a2.133 2.133 0 1 1 4.247 0 2.134 2.134 0 1 1-4.247 0zm3.333 0c0-.976-.438-1.547-1.208-1.547-.772 0-1.207.571-1.207 1.547 0 .984.435 1.55 1.207 1.55.77 0 1.208-.57 1.208-1.55zM84.67 10.195h.855v.715h.066a1.348 1.348 0 0 1 1.344-.802 1.465 1.465 0 0 1 1.559 1.675v2.915h-.889v-2.692c0-.724-.314-1.084-.972-1.084a1.033 1.033 0 0 0-1.075 1.141v2.635h-.889zM93.515 9.074v1.141h.976v.749h-.976v2.315c0 .472.194.679.637.679a2.967 2.967 0 0 0 .339-.021v.74a2.916 2.916 0 0 1-.484.046c-.988 0-1.381-.348-1.381-1.216v-2.543h-.715v-.749h.715V9.074zM95.705 8.437h.88v2.481h.07a1.386 1.386 0 0 1 1.374-.806 1.483 1.483 0 0 1 1.55 1.679v2.907h-.889V12.01c0-.72-.335-1.084-.963-1.084a1.052 1.052 0 0 0-1.134 1.142v2.63h-.888zM104.761 13.482a1.828 1.828 0 0 1-1.95 1.303 2.045 2.045 0 0 1-2.081-2.325 2.077 2.077 0 0 1 2.076-2.352c1.253 0 2.009.856 2.009 2.27v.31h-3.18v.05a1.19 1.19 0 0 0 1.2 1.29 1.08 1.08 0 0 0 1.07-.546zm-3.126-1.451h2.275a1.086 1.086 0 0 0-1.109-1.167 1.152 1.152 0 0 0-1.166 1.167z"/></g></g></svg>

Nit: the groups in this svg aren't necessary, and the fills could be specified directly on the `path` elements. It would also be nice to at least move the `path` elements to their own lines so that the file becomes a bit more readable. It's possible `svgo` could help with this.

::: browser/extensions/pocket/content/panels/js/saved.js:385
(Diff revision 1)
> +        if (initobj.ho2 && initobj.ho2 !== 'control'
> +            && !initobj.accountState.has_mobile
> +            && myself.savedUrl.indexOf('getpocket.com') === -1) {

FWIW, prevailing style in Firefox is loose-equals (so == rather than === and != rather than !==) and trailing rather than leading operators in multi-line expressions.

I don't know to what degree we want to get the pocket system add-on code to match Firefox or not...

::: browser/extensions/pocket/content/panels/js/saved.js:394
(Diff revision 1)
> +            myself.freezeHeight = true;
> +        }
> +
>          myself.fillUserTags();
>          if (myself.suggestedTagsLoaded) {
>              myself.startCloseTimer();

Should we still start the auto-close timer if we're showing users the "hey, send this to mobile" upsell?

::: browser/extensions/pocket/content/panels/js/sendtomobile.js:78
(Diff revision 1)
> +
> +    /**
> +     * Public functions
> +     */
> +    return {
> +        create

Nit: trailing comma please, so we don't need to touch this line if we want to add other properties.

::: browser/extensions/pocket/content/panels/js/sendtomobile.js:100
(Diff revision 1)
> +    if (resize)
> +        query.resize = resize;
> +    if (fallback)
> +        query.f = fallback;
> +
> +    return 'https://d33ypg4xwx0n86.cloudfront.net/direct?' + $.param(query);

You can get the md5 hash via XPCOM instead. The current method requires sending these URLs along the internet, which is going to be slow as well as a potential privacy issue, depending on when this gets called.

::: browser/extensions/pocket/content/panels/tmpl/ho2/ho2_sharebutton_v2.handlebars:2
(Diff revision 1)
> +<div class="pkt_ext_detail pkt_ext_saved_sendtomobile">
> +    <div id="pkt_ext_sendtomobile_button" class="pkt_ext_button">

The fact that you're using a `<div>` to represent a button means these buttons aren't usable for users who use assistive technology (like screenreaders) and can't be used with the keyboard.

Please can we use 'real' buttons instead, or if that's a problem with styling, at least an `<a href="#">`, which gets you better focus, keyboard and AT handling?

::: browser/extensions/pocket/content/panels/tmpl/saved_tmplogin.handlebars:4
(Diff revision 1)
> +<div class="pkt_ext_detail pkt_ext_saved_tmplogin pkt_shaded_background">
> +    <div class="pkt_ext_indent_border_left">
> +        <p>You are using Pocket without an account.<br />Sign up to back up your saved items.</p>
> +        <p><a href="https://help.getpocket.com/article/1129-using-pocket-without-an-account-in-firefox?s=ghost_upsell" target="_blank">Learn more</a>

Nit: missing `</p>` (and yes, that's valid, but you may as well add it...)

::: browser/extensions/pocket/content/pktApi.jsm:728
(Diff revision 1)
>          getSuggestedTagsForItem,
>          getSuggestedTagsForURL,
>          getSignupPanelTabTestVariant,
>          retrieve,
> +        getArticleInfo,
> +        getMobileDownload

Nit: trailing comma please.
Attachment #8975531 - Flags: review?(gijskruitbosch+bugs)

Comment 7

10 months ago
mozreview-review-reply
Comment on attachment 8975531 [details]
Bug 1451840 - Add button to Pocket doorhanger to request mobile app

https://reviewboard.mozilla.org/r/243786/#review249944

> This isn't RTL-friendly and should probably be using logical-start margins/paddings/borders instead (and possibly switch the name to `border_start` or something like that) to avoid looking strange in RTL locales like Hebrew and Arabic.

Oh, and obviously this doesn't matter much while this is en-US only, but I think it's probably easier to address this immediately and make sure the direction-agnostic styles work with en-US, than to retrofit patches later once we want to roll this out. :-)
(Assignee)

Comment 8

10 months ago
(In reply to :Gijs (he/him) from comment #7)
> Out of interest, what does `ho2` stand for?
It's the internal code-name for the experiment. The PM came up with it.


> But also, could you just animate the colour directly instead of the bg scroll you're using here to make the background colour pulse?
We go for the bg scroll since it's a gradient that slides and gradients aren't animatable. 


> Should we still start the auto-close timer if we're showing users the "hey, send this to mobile" upsell?
It shouldn't interrupt the current save process, it's more of a passive option.


> You can get the md5 hash via XPCOM instead. The current method requires sending these URLs along the internet, which is going to be slow as well as a potential privacy issue, depending on when this gets called.
This is a standard utility function that we use across all of our applications. I spoke with the owner of the Image Cache and he says that a much older version used the MD5, but not anymore.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 11

10 months ago
mozreview-review
Comment on attachment 8975531 [details]
Bug 1451840 - Add button to Pocket doorhanger to request mobile app

https://reviewboard.mozilla.org/r/243786/#review250880

r=me with these csets merged.

In mozreview, we normally merge review fixes into the original csets to which they belong. The autoland stuff doesn't support doing a merge-when-landing, and so the easiest is if I r+ this cset and you fold the second cset in the first.

If you've used ./mach bootstrap to set up your `hg` environment, something like the following should work:

```
hg up -r 5c62178e2579aa72da14a37a7c9cd5c6f702b6a6
hg histedit
```

and you'll get a screen similar to git's interactive rebase. You can just change the 'pick' in front of the second cset to `fold` and it'll fold the two. There's probably an `hg fold` way of doing this, but `histedit` is pretty useful anyway and makes it more obvious what's happening (I think).

Anyway, then you can just push the modified cset to mozreview and it should figure out what's happened and obsolete 1 of the csets and hopefully carry over this r+. :-)
Attachment #8975531 - Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

10 months ago
Attachment #8976702 - Attachment is obsolete: true

Comment 13

10 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2eba9608447d
Add button to Pocket doorhanger to request mobile app r=Gijs

Comment 14

10 months ago
Backed out changeset 2eba9608447d (bug 1451840) for linting failure. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=179137538&repo=autoland&lineNumber=258

task 2018-05-18T08:13:39.027Z] 
[task 2018-05-18T08:13:39.027Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/main.js:418:17 | Expected method shorthand. (object-shorthand)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/main.js:421:17 | Expected method shorthand. (object-shorthand)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/main.js:433:17 | Expected method shorthand. (object-shorthand)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/main.js:436:17 | Expected method shorthand. (object-shorthand)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/saved.js:385:43 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/saved.js:387:16 | use .includes instead of .indexOf (mozilla/use-includes-instead-of-indexOf)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/saved.js:387:40 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/saved.js:400:9 | 'PKT_SENDTOMOBILE' is not defined. (no-undef)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:16:9 | Closing curly brace does not appear on the same line as the subsequent block. (brace-style)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:23:84 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:27:9 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:27:11 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:27:40 | 'Handlebars' is not defined. (no-undef)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:31:9 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:31:11 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:32:13 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:32:15 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:34:13 | 'thePKT_SAVED' is not defined. (no-undef)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:34:38 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.190Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:36:21 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:36:23 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:36:36 | 'Handlebars' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:37:21 | 'thePKT_SAVED' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:37:46 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:37:63 | Expected property shorthand. (object-shorthand)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:38:17 | Closing curly brace does not appear on the same line as the subsequent block. (brace-style)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:40:21 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:40:23 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:40:36 | 'Handlebars' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:41:21 | 'thePKT_SAVED' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:41:46 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:41:63 | Expected property shorthand. (object-shorthand)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:49:9 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:49:11 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:49:28 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:52:20 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:54:13 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:54:15 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:54:30 | 'Handlebars' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:55:13 | 'thePKT_SAVED' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:55:38 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:56:13 | 'thePKT_SAVED' is not defined. (no-undef)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:56:38 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:59:9 | Closing curly brace does not appear on the same line as the subsequent block. (brace-style)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:60:25 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.191Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:62:13 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:62:15 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:62:30 | 'Handlebars' is not defined. (no-undef)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:63:13 | 'thePKT_SAVED' is not defined. (no-undef)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:63:38 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:64:9 | Closing curly brace does not appear on the same line as the subsequent block. (brace-style)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:65:25 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:67:13 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:67:15 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:67:30 | 'Handlebars' is not defined. (no-undef)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:68:13 | 'thePKT_SAVED' is not defined. (no-undef)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:68:38 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:92:9 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:99:5 | Function 'getImageCacheUrl' expected no return value. (consistent-return)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:99:12 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/panels/js/sendtomobile.js:99:62 | '$' is not defined. (no-undef)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/pktApi.jsm:363:80 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/pktApi.jsm:363:127 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/pktApi.jsm:384:19 | Strings must use doublequote. (quotes)
[task 2018-05-18T08:18:31.192Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/extensions/pocket/content/pktApi.jsm:405:19 | Strings must use doublequote. (quotes)
[taskcluster 2018-05-18 08:18:31.633Z] === Task Finished ===
[taskcluster 2018-05-18 08:18:31.633Z] Unsuccessful task run with exit code: 1 completed in 556.077 seconds

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2eba9608447d0e8d2572b292d8d733645d698286

Backout:
https://hg.mozilla.org/integration/autoland/rev/9dd8b6f551abb9d1e3ecb3b68eba4ee683371376
Flags: needinfo?(anthony)
Comment hidden (mozreview-request)

Comment 16

10 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f5cbef593cc1
Add button to Pocket doorhanger to request mobile app r=Gijs
(Assignee)

Updated

10 months ago
Flags: needinfo?(anthony)
Comment hidden (mozreview-request)

Comment 19

10 months ago
Anthony is clearing this up. :-)

Anthony, do let me know if you need any help pushing to try / relanding this.
Assignee: nobody → anthony
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)

Comment 20

10 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8ab9b8ed8617
Add button to Pocket doorhanger to request mobile app r=Gijs

Comment 21

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8ab9b8ed8617
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.