Closed Bug 1382520 Opened 7 years ago Closed 7 years ago

Place visual assets of onboarding tours of version 57

Categories

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

enhancement

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- verified

People

(Reporter: rexboy, Assigned: Fischer)

References

Details

(Whiteboard: [photon-onboarding])

Attachments

(6 files, 4 obsolete files)

We have these visual assets to be updated upon available:
- Fast&Modern
- Library tour
- Screenshot

For each of them, we need description figure in tour page, and icon tip in tour notification.
Summary: Place visual assets of onboarding tours in version 57. → Place visual assets of onboarding tours of version 57
Target Milestone: --- → Firefox 57
Flags: qe-verify+
QA Contact: jwilliams
Blocks: 1366056
Sean created new illustrations for each of the overlay panels: 
*Add-ons
*Customize
*Default Browser
*Library
*Performance
*Private Browsing
*Screenshots
*Search
*Sync
Sean also created new illustrations for each of the tour notifications:
*Add-ons
*Customize
*Default Browser
*Library
*Performance
*Private Browsing
*Screenshots
*Search
*Sync
I'm still waiting for new navigation icons for:
*Performance
*Default Browser
Here are the performance and default browser navigation icons for the overlay
Assignee: nobody → fliu
Status: NEW → ASSIGNED
Priority: P2 → P1
Hi Verid,
Some items to check with you:

(In reply to Verdi [:verdi] from comment #1)
> Created attachment 8895581 [details]
> 57-overlay-illustrations.zip
> 
> Sean created new illustrations for each of the overlay panels: 
> *Add-ons
> *Customize
> *Default Browser
> *Library
> *Performance
> *Private Browsing
> *Screenshots
> *Search
> *Sync
1. Add-ons only gets one addons.png not svg like other tours. But that addons.png looks like the same as the currently addons image for 56. Is that 57 Add-ons image is that same as the current 56 one?
2. Only see imges for the Search tour. Is that the current 56 Search tour shares the same images with the 57 Single-Search 
tour?

(In reply to Verdi [:verdi] from comment #4)
> Created attachment 8895610 [details]
> performance-default-nav-icons.zip
> 
> Here are the performance and default browser navigation icons for the overlay
3. Didn't see the navigation icon for the Library tour. We need that.
4. Didn't see the navigation icon for the Screenshots tour. We need that.
Flags: needinfo?(mverdi)
Attached image addons.svg
(In reply to Fischer [:Fischer] from comment #5)
> 1. Add-ons only gets one addons.png not svg like other tours. But that
> addons.png looks like the same as the currently addons image for 56. Is that
> 57 Add-ons image is that same as the current 56 one?

Sorry - that was my careless packaging. Here is the svg file. It is slightly different than the current 56 image.
Flags: needinfo?(mverdi)
(In reply to Fischer [:Fischer] from comment #5)
> 2. Only see imges for the Search tour. Is that the current 56 Search tour
> shares the same images with the 57 Single-Search 
> tour?
> 

search.svg is for the 57 Single-Search tour. It's slightly different than the one used in the 56 tour.

> (In reply to Verdi [:verdi] from comment #4)
> > Created attachment 8895610 [details]
> > performance-default-nav-icons.zip
> > 
> > Here are the performance and default browser navigation icons for the overlay
> 3. Didn't see the navigation icon for the Library tour. We need that.
> 4. Didn't see the navigation icon for the Screenshots tour. We need that.

Those were previously attached to Bug 1360378 - here's the link https://bugzilla.mozilla.org/attachment.cgi?id=8886895 Sorry for the confusion.
Attached image addons_notification.png (obsolete) —
Attached image addons_overlay.png (obsolete) —
Attached image customize_notification.png (obsolete) —
Attached image customize_overlay.png (obsolete) —
Attachment #8896154 - Attachment is obsolete: true
Attachment #8896155 - Attachment is obsolete: true
Attachment #8896156 - Attachment is obsolete: true
Attachment #8896158 - Attachment is obsolete: true
(In reply to Fischer [:Fischer] from comment #13)
> Created attachment 8896166 [details]
> Bug 1382520 - Place visual assets of onboarding tours of version 57,
> 
> Review commit: https://reviewboard.mozilla.org/r/167424/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/167424/
Hi Mossop,
If you want to see the new images, please visit the link: attachment 8896160 [details]: Bug_1382520_Onboarding_New_Visual_Assest
Thanks
Comment on attachment 8896166 [details]
Bug 1382520 - Place visual assets of onboarding tours of version 57,

https://reviewboard.mozilla.org/r/167424/#review172760

::: browser/extensions/onboarding/content/onboarding.css:347
(Diff revision 1)
>  .onboarding-tour-action-button:disabled {
>    opacity: 0.5;
>  }
>  
>  /* Tour Icons */
> -#onboarding-tour-search,
> +#onboarding-tour-search {

we can remove search tour and just keep the singlesearch now, but we can do that in followup bug
Comment on attachment 8896166 [details]
Bug 1382520 - Place visual assets of onboarding tours of version 57,

https://reviewboard.mozilla.org/r/167424/#review172856
Attachment #8896166 - Flags: review?(dtownsend) → review+
(In reply to Fred Lin [:gasolin] from comment #15)
> Comment on attachment 8896166 [details]
> Bug 1382520 - Place visual assets of onboarding tours of version 57,
> 
> https://reviewboard.mozilla.org/r/167424/#review172760
> 
> ::: browser/extensions/onboarding/content/onboarding.css:347 (Diff revision 1)
> >  /* Tour Icons */
> > -#onboarding-tour-search,
> > +#onboarding-tour-search {
> 
> we can remove search tour and just keep the singlesearch now, but we can do that in followup bug
Right, we should be able to remove the Search tour since that is a 56-only tour.
Didn't do that because only focused on the images update while handling the bug.
Like you said, we probably could do that in Bug 1366056 - Show existing users the 57 tour and update new user tour, which relates to that as well.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/33901272db44
Place visual assets of onboarding tours of version 57, r=mossop
Keywords: checkin-needed
Backed out for packaging failure:

https://hg.mozilla.org/integration/autoland/rev/54f24c933fda8ffe9398c7908462e4bec1465c4d

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=33901272db44028fa7ac99e962b8bcfe9023847b&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122877630&repo=autoland

[task 2017-08-13T21:09:39.397747Z] 21:09:39     INFO -  WARNING: Found 33 duplicated files taking 83989 bytes (uncompressed)
[task 2017-08-13T21:09:39.397919Z] 21:09:39     INFO -  ERROR: The following duplicated files are not allowed:
[task 2017-08-13T21:09:39.398102Z] 21:09:39     INFO -  browser/features/onboarding@mozilla.org/chrome/content/img/icons_search-colored.svg
[task 2017-08-13T21:09:39.398280Z] 21:09:39     INFO -  browser/features/onboarding@mozilla.org/chrome/content/img/icons_singlesearch-colored.svg
[task 2017-08-13T21:09:39.398453Z] 21:09:39     INFO -  browser/features/onboarding@mozilla.org/chrome/content/img/icons_search.svg
[task 2017-08-13T21:09:39.398635Z] 21:09:39     INFO -  browser/features/onboarding@mozilla.org/chrome/content/img/icons_singlesearch.svg
[task 2017-08-13T21:09:39.398824Z] 21:09:39     INFO -  /home/worker/workspace/build/src/toolkit/mozapps/installer/packager.mk:39: recipe for target 'stage-package' failed
[task 2017-08-13T21:09:39.398983Z] 21:09:39     INFO -  gmake[7]: *** [stage-package] Error 1
Flags: needinfo?(fliu)
There's an image checker to make sure no duplicate images, so we'd better use same image for both search/singlesearch illustration. It's fine since we'll remove the search tour soon.
Flags: needinfo?(fliu)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bd9e37c5008d
Place visual assets of onboarding tours of version 57, r=mossop
Keywords: checkin-needed
Backed out for failing mochitest browser/base/content/test/static/browser_all_files_referenced.js:

https://hg.mozilla.org/integration/autoland/rev/f667fdab3ac0b9e10ce396f8cdc33499ced1ffbf

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bd9e37c5008db816cf3cc79f79bb099586336d2f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122905708&repo=autoland

21:00:16     INFO - TEST-START | browser/base/content/test/static/browser_all_files_referenced.js
21:00:17     INFO - GECKO(1751) | JavaScript error: resource://activity-stream/data/content/activity-stream.bundle.js, line 736: ReferenceError: addMessageListener is not defined
21:00:25     INFO - TEST-INFO | started process screencapture
21:00:25     INFO - TEST-INFO | screencapture: exit 0
21:00:25     INFO - Buffered messages logged at 21:00:16
21:00:25     INFO - Entering test bound checkAllTheFiles
21:00:25     INFO - Buffered messages logged at 21:00:21
21:00:25     INFO - Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 173}]
21:00:25     INFO - Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 173}]
21:00:25     INFO - Console message: [JavaScript Error: "ReferenceError: addMessageListener is not defined" {file: "resource://activity-stream/data/content/activity-stream.bundle.js" line: 736}]
21:00:25     INFO - Buffered messages finished
21:00:25     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | there should be no unreferenced files - Got 4, expected 0
21:00:25     INFO - Stack trace:
21:00:25     INFO - chrome://mochikit/content/browser-test.js:test_is:1002
21:00:25     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_all_files_referenced.js:checkAllTheFiles:600
21:00:25     INFO - Not taking screenshot here: see the one that was previously logged
21:00:25     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | unreferenced file: resource://onboarding/img/figure_screenshots.svg - 
21:00:25     INFO - Stack trace:
21:00:25     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_all_files_referenced.js:checkAllTheFiles:602
21:00:25     INFO - Not taking screenshot here: see the one that was previously logged
21:00:25     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | unreferenced file: resource://onboarding/img/icons_screenshots-colored.svg - 
21:00:25     INFO - Stack trace:
21:00:25     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_all_files_referenced.js:checkAllTheFiles:602
21:00:25     INFO - Not taking screenshot here: see the one that was previously logged
21:00:25     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | unreferenced file: resource://onboarding/img/icons_screenshots-notification.svg - 
21:00:25     INFO - Stack trace:
21:00:25     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_all_files_referenced.js:checkAllTheFiles:602
21:00:25     INFO - Not taking screenshot here: see the one that was previously logged
21:00:25     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | unreferenced file: resource://onboarding/img/icons_screenshots.svg -
Flags: needinfo?(fliu)
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #23)
> 21:00:25     INFO - TEST-UNEXPECTED-FAIL |
> browser/base/content/test/static/browser_all_files_referenced.js |
> unreferenced file: resource://onboarding/img/icons_screenshots-colored.svg - 
> 21:00:25     INFO - Stack trace:
> 21:00:25     INFO -
> chrome://mochitests/content/browser/browser/base/content/test/static/
> browser_all_files_referenced.js:checkAllTheFiles:602
> 21:00:25     INFO - Not taking screenshot here: see the one that was
> previously logged
> 21:00:25     INFO - TEST-UNEXPECTED-FAIL |
> browser/base/content/test/static/browser_all_files_referenced.js |
> unreferenced file:
> resource://onboarding/img/icons_screenshots-notification.svg - 
> 21:00:25     INFO - Stack trace:
> 21:00:25     INFO -
> chrome://mochitests/content/browser/browser/base/content/test/static/
> browser_all_files_referenced.js:checkAllTheFiles:602
> 21:00:25     INFO - Not taking screenshot here: see the one that was
> previously logged
> 21:00:25     INFO - TEST-UNEXPECTED-FAIL |
> browser/base/content/test/static/browser_all_files_referenced.js |
> unreferenced file: resource://onboarding/img/icons_screenshots.svg -
This is no good. Should add the Screenshots tour images in the Bug 1371538 - Should add the Screenshots tour in the onBoarding overlay.
Flags: needinfo?(fliu)
Sorry for the issue. This time should be better. TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dec52a0554bc
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bdddd267a26e
Place visual assets of onboarding tours of version 57, r=mossop
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bdddd267a26e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
I have verified this bug is fixed on today's nightly.
Status: RESOLVED → VERIFIED
I can confirm this issue is fixed on Fx 57.0b7 as well.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: