1787387 - No feature callout tour shown if tab sync is already set up

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Ed Lee :Mardak]

The first screen of the tour looks for #tabpickup-steps but that doesn't exist if sync is already set up.

TypeError: can't access property "insertAdjacentElement", parent is null

Seems like there's also some issue of the automatic behavior to remove a screen without the parent to move to the next screen.

Comment 1

[Meg Viar [:mviar]]

Ah, I didn't realize this screen should also be shown conditionally like the colorways screen. We may want to handle it the way we're handling targeting screens with no colorways in bug 1786646.

Comment 2

[Vtay]

We still want to show the feature callout tour even if tab sync is already set up

Comment 3

[Meg Viar [:mviar]]

We'll just want to change the #tabpickup-steps selector (possibly to a different parent container?) and still show the initial Feature Callout product tour tab pickup screen even if it's already enabled.

If there's no parent container that's present when sync is both enabled and not enabled, we may need to add a class to the respective tab pickup containers that's consistent across the two states and use that as our parent selector.

Comment 4

[Meg Viar [:mviar]]

Attachment: Bug 1787387 - No feature callout tour shown if tab sync is already set up

Comment 5

[Meg Viar [:mviar]]

@Venetia to @mardak's question in my patch - if a callout message was misconfigured or its parent page had changed in some way so that the element the callout was supposed to point to is missing, what's the expected behavior? Should we just advance to the next screen (if there is one)? I can imagine cases where that might not be desirable, like if the messages build on each other in some way in a sequence. It may also be confusing in the step indicator would show that a step had been "missed" (maybe even more confusing with screen reader output). We do have a way to explicitly handle parent elements that are expected to be missing sometimes, like we do with Colorways. So, this would just be for cases where something unexpected is going on.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af3277c93023
No feature callout tour shown if tab sync is already set up r=Mardak

Comment 7

[Vtay]

I am expecting the possibility of this happening to be very low. Is it possible not to show the tour at all? I rather not show the tour vs giving the user a bad experience.

Comment 8

[Meg Viar [:mviar]]

Thanks for the feedback @vtay, with a few changes, we could confirm the presence of all parent elements before the tour is launched. That doesn't solve for the case of some use action resulting in an expected element not being present (say something dismiss-able or that changes to a different element after interaction). In those cases would you just end the tour?

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/af3277c93023

Comment 10

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Nightly (106.0a1 Build ID - 20220830210405) installed on Windows 10 x64, macOS 11.6.5, and Linux Mint 20.2 x64. Now, I can confirm that, after connecting to sync, the first screen of the callout points to the "Tab pickup" section.

Comment 11

[Meg Viar [:mviar]]

Hey @vtay, just following up with an NI on question above

Comment 12

[Vtay]

As per our chat, in the case where an expected element is not being present, we will not continue showing the tour (even if the tour is underway). We should have a way to track how often this happens and we can make a decision is this happens frequently