1397701 - Should clean up UITour after closing the onboarding overlay by clicking the Skip-Tour button

Status: VERIFIED FIXED

(Firefox :: Tours, defect, P1)

Description

[Fischer [:Fischer]]

STR:
1. Open the onboarding tour overlay
2. Navigate to the Library tour
3. Click the Library tour action button to trigger the UITour highlight on the Library button
4. Click the Skip-Tour button on the onboarding overlay

p.s we could use any tour which would highlight button in the step 2 and 3.

Expected Result:
The overlay closes and the UITour highlight on the Library button closes as well

Actual Result:
The overlay closes but the UITour highlight on the Library button doesn't close

Root Cause:
The Onboarding cleans up UITour at [1] when seeing the overlay or the overlay-close button clicked to close the overlay. We should consider the case of the Skip-Tour button too. However, now there are 3 buttons which should be considered, in case of the future change, such as adding one more button, removing another button etc. We probably need a more general way to clean up UITour while the onboarding overlay closed, otherwise, it is very easy to forget the thing. And some test about this behaviour is needed too.

[1] http://searchfox.org/mozilla-central/rev/67f38de2443e6b613d874fcf4d2cd1f2fc3d5e97/browser/extensions/onboarding/content/onboarding-tour-agent.js#61

Comment 1

[Fischer [:Fischer]]

The bug 1394730 would handle this bug together.

Comment 2

[Fischer [:Fischer]]

Attachment: Bug 1397701 - Should clean up UITour after closing the onboarding overlay by clicking the Skip-Tour button,

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

Comment 3

[Fischer [:Fischer]]

(In reply to Fischer [:Fischer] from comment #1)
> The bug 1394730 would handle this bug together.
Per bug 1394730 comment 6, move the patch back to this bug

Comment 4

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8906486 [details]
Bug 1397701 - Should clean up UITour after closing the onboarding overlay by clicking the Skip-Tour button,

https://reviewboard.mozilla.org/r/178246/#review183192

::: browser/extensions/onboarding/content/onboarding-tour-agent.js:90
(Diff revision 1)
>    }
>  });
> +let overlayObserver = new MutationObserver(mutations => {
> +  // Clean up UITour if a user tries to close the dialog.
> +  if (!overlay.classList.contains("onboarding-opened")) {
> +    cleanupUITour();

same as comment in bug 1394730

Comment 5

[Fischer [:Fischer]]

Comment on attachment 8906486 [details]
Bug 1397701 - Should clean up UITour after closing the onboarding overlay by clicking the Skip-Tour button,

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

Comment 6

[Fischer [:Fischer]]

Comment on attachment 8906486 [details]
Bug 1397701 - Should clean up UITour after closing the onboarding overlay by clicking the Skip-Tour button,

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

Comment 7

[Fischer [:Fischer]]

TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e41816a490f9c5335a84d73a2b010ed3c1702f0c

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5de9bbbefd90
Should clean up UITour after closing the onboarding overlay by clicking the Skip-Tour button, r=rexboy

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/5de9bbbefd90

Comment 10

[Justin [:JW_SoftvisionQA]]

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

Comment 11

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

I reproduced this issue using Fx 57.0a1 (2017-09-07) on Windows 10 x64. I can confirm this issue is fixed, I verified using Fx 57.0b7, on Windows 10 x64, Ubuntu 14.04 LTS and mac OS X 10.12.6.