1392469 - [Onboarding] remove fox icon and speech bubble from notification

Status: VERIFIED FIXED

(Firefox :: New Tab Page, enhancement, P1)

Description

[[e-mail]]

* remove fox icon and speech bubble from notification
* remove all notification illustrations

Comment 1

[[e-mail]]

Attachment: Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

Review commit: https://reviewboard.mozilla.org/r/171968/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171968/

Comment 2

[[e-mail]]

Attachment: Bug 1392469 - [Onboarding] remove all notification illustrations;

Review commit: https://reviewboard.mozilla.org/r/171970/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171970/

Comment 3

[[e-mail]]

bug 1371538 doesn't need to include the illustration for notification after this UI update is landed.

Comment 4

[[e-mail]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

https://reviewboard.mozilla.org/r/171968/#review177194

::: browser/extensions/onboarding/content/onboarding.css:536
(Diff revision 1)
> -@media all and (max-width: 960px) {
> -  #onboarding-notification-icon {
> -    display: none;
> -  }
> -}
>  @media all and (max-width: 720px) {

I think we don't need this either but can be done in bug 1392552

Comment 5

[[e-mail]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171968/diff/1-2/

Comment 6

[[e-mail]]

Attachment: Bug 1392469 - [Onboarding] remove all notification illustrations;

Review commit: https://reviewboard.mozilla.org/r/171982/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171982/

Comment 7

[[e-mail]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171968/diff/2-3/

Comment 8

[[e-mail]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171968/diff/3-4/

Comment 9

[[e-mail]]

Comment on attachment 8900584 [details]
Bug 1392469 - [Onboarding] remove all notification illustrations;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171982/diff/1-2/

Comment 10

[[e-mail]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171968/diff/4-5/

Comment 11

[[e-mail]]

Comment on attachment 8900584 [details]
Bug 1392469 - [Onboarding] remove all notification illustrations;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171982/diff/2-3/

Comment 12

[Fischer [:Fischer]]

Comment on attachment 8900584 [details]
Bug 1392469 - [Onboarding] remove all notification illustrations;

https://reviewboard.mozilla.org/r/171982/#review177304

Comment 13

[[e-mail]]

Comment on attachment 8900584 [details]
Bug 1392469 - [Onboarding] remove all notification illustrations;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171982/diff/3-4/

Comment 14

[[e-mail]]

found icon64.png fit 64x64 size well, so replace it instead of [e-mail]

Comment 15

[Fischer [:Fischer]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

https://reviewboard.mozilla.org/r/171968/#review177310

r+ with issues addressed, thanks

::: browser/extensions/onboarding/content/onboarding.js
(Diff revision 5)
>          <button id="onboarding-notification-action-btn"></button>
>        </section>
>        <button id="onboarding-notification-close-btn" class="onboarding-close-btn"></button>
>      `;
> -    let toolTip = this._bundle.formatStringFromName(
> -      this._tourType === "new" ? "onboarding.notification-icon-tool-tip" :

If these strings were no longer needed, please remove them from the properties file.

::: browser/extensions/onboarding/test/browser/browser_onboarding_accessibility.js:47
(Diff revision 5)
>    await ContentTask.spawn(tab.linkedBrowser, {}, function() {
>      let doc = content.document;
>      let footer = doc.getElementById("onboarding-notification-bar");
> -    let icon = doc.getElementById("onboarding-notification-icon")
> +    let title = doc.getElementById("onboarding-notification-tour-title");
> +
> +    is(footer.getAttribute("aria-labelledby"), title.id,

doc.getElementById("onboarding-notification-tour-title").id is always "onboarding-notification-tour-title" so maybe title.id could be removed.

Comment 16

[[e-mail]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

https://reviewboard.mozilla.org/r/171968/#review177310

> If these strings were no longer needed, please remove them from the properties file.

nice catch! updated

> doc.getElementById("onboarding-notification-tour-title").id is always "onboarding-notification-tour-title" so maybe title.id could be removed.

updated

Comment 17

[[e-mail]]

Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171968/diff/5-6/

Comment 18

[[e-mail]]

Comment on attachment 8900584 [details]
Bug 1392469 - [Onboarding] remove all notification illustrations;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171982/diff/4-5/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/baeaa44166fb
[Onboarding] remove fox icon and speech bubble from notification;r=Fischer
https://hg.mozilla.org/integration/autoland/rev/5da4c3f0fd7c
[Onboarding] remove all notification illustrations;r=Fischer

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/baeaa44166fb
https://hg.mozilla.org/mozilla-central/rev/5da4c3f0fd7c

Comment 21

[Justin [:JW_SoftvisionQA]]

I have verified that this issue is fixed on today's nightly.

Comment 22

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I can confirm the intended behavior is respected on beta. I verified using Fx 57.0b7 on Windows 10 x64, Ubuntu 14.04 LTS and macOS X 10.12.6.