1374544 - Inconsistent onboarding experience depending on if a user accesses the tour from about:home or about:newtab

Status: NEW

(Firefox :: General, defect, P5)

Description

[chsiang]

There are discrepancies between below behaviors:(recorded in the video)

1. On about:home
Click addons under the hamburger menu -> open about:addons in a new tab
Click preferences under the hamburger menu --> open about:preferences in a new tab

2. Change the about:home tab to about:newtab
Click addons under the hamburger menu -> open about:addons in a new tab
Click preferences under the hamburger menu --> open about:preferences in a new tab

3. Open about:newtab in a different tab
Click addons under the hamburger menu -> open about:addons in the existing tab
Click preferences under the hamburger menu --> open about:preferences in the existing tab

#3 is not our desired user experience and will make it difficult for a user to get back to the tour if they want to.

Comment 1

[chsiang]

Hi Michael,
Please take a look at this bug (video attached) and confirm that the desired behavior is to open about:addons in a new tab.

Thanks.
Cindy

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

UITour.jsm asks our common function to opne the Preferences page. It will call into other functions; eventually there is a |isBlankPageURL(aURL)| that make the decision and allow the new tab page to be overwritten (the other page that will get overwritten is the about:blank).

http://searchfox.org/mozilla-central/rev/2bcd258281da848311769281daf735601685de2d/browser/base/content/utilityOverlay.js#44
http://searchfox.org/mozilla-central/rev/2bcd258281da848311769281daf735601685de2d/browser/base/content/browser.js#6709
http://searchfox.org/mozilla-central/rev/2bcd258281da848311769281daf735601685de2d/browser/base/content/browser.js#8066
http://searchfox.org/mozilla-central/rev/2bcd258281da848311769281daf735601685de2d/browser/base/content/utilityOverlay.js#725

This was there when the newtab was implemented (bug 455553) according to blame log. It would be trivial to get rid of a line of code but I have not look into what will break by removing about:newtab from that function.

Comment 3

[Verdi [:verdi] Best to slack me]

(In reply to chsiang from comment #1)
> Hi Michael,
> Please take a look at this bug (video attached) and confirm that the desired
> behavior is to open about:addons in a new tab.
> 
> Thanks.
> Cindy

Ok got it. If possible, it would be great if we could fix this. I would think it would be a P3.

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #2)
> UITour.jsm asks our common function to opne the Preferences page. It will
> call into other functions; eventually there is a |isBlankPageURL(aURL)| that
> make the decision and allow the new tab page to be overwritten (the other
> page that will get overwritten is the about:blank).
> 
> http://searchfox.org/mozilla-central/rev/
> 2bcd258281da848311769281daf735601685de2d/browser/base/content/utilityOverlay.
> js#44
> http://searchfox.org/mozilla-central/rev/
> 2bcd258281da848311769281daf735601685de2d/browser/base/content/browser.js#6709
> http://searchfox.org/mozilla-central/rev/
> 2bcd258281da848311769281daf735601685de2d/browser/base/content/browser.js#8066
> http://searchfox.org/mozilla-central/rev/
> 2bcd258281da848311769281daf735601685de2d/browser/base/content/utilityOverlay.
> js#725
> 
> This was there when the newtab was implemented (bug 455553) according to
> blame log. It would be trivial to get rid of a line of code but I have not
> look into what will break by removing about:newtab from that function.

I'm unclear why we're talking about opening the preference page unless that affects how opening about:addons works.

Comment 4

[chsiang]

(In reply to Verdi [:verdi] from comment #3)
> (In reply to chsiang from comment #1)
> > Hi Michael,
> > Please take a look at this bug (video attached) and confirm that the desired
> > behavior is to open about:addons in a new tab.
> > 
> > Thanks.
> > Cindy
> 
> Ok got it. If possible, it would be great if we could fix this. I would
> think it would be a P3.
> 
> (In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment
> #2)
> > UITour.jsm asks our common function to opne the Preferences page. It will
> > call into other functions; eventually there is a |isBlankPageURL(aURL)| that
> > make the decision and allow the new tab page to be overwritten (the other
> > page that will get overwritten is the about:blank).
> > 
> > http://searchfox.org/mozilla-central/rev/
> > 2bcd258281da848311769281daf735601685de2d/browser/base/content/utilityOverlay.
> > js#44
> > http://searchfox.org/mozilla-central/rev/
> > 2bcd258281da848311769281daf735601685de2d/browser/base/content/browser.js#6709
> > http://searchfox.org/mozilla-central/rev/
> > 2bcd258281da848311769281daf735601685de2d/browser/base/content/browser.js#8066
> > http://searchfox.org/mozilla-central/rev/
> > 2bcd258281da848311769281daf735601685de2d/browser/base/content/utilityOverlay.
> > js#725
> > 
> > This was there when the newtab was implemented (bug 455553) according to
> > blame log. It would be trivial to get rid of a line of code but I have not
> > look into what will break by removing about:newtab from that function.
> 
> I'm unclear why we're talking about opening the preference page unless that
> affects how opening about:addons works.

I was just assuming opening about:addons and about:preferences under the hamburger menu follows the same logic.

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to chsiang from comment #4)
> (In reply to Verdi [:verdi] from comment #3)
> > I'm unclear why we're talking about opening the preference page unless that
> > affects how opening about:addons works.
> 
> I was just assuming opening about:addons and about:preferences under the
> hamburger menu follows the same logic.

My fault. Too much "Preferences" ringing in my mind, but yeah opening about:addons depends on the same isTabEmpty() and isBlankPageURL() to decide whether to overwrite the current tab.

http://searchfox.org/mozilla-central/rev/714606a8145636d93b116943d3a65a6a49d2acf8/browser/base/content/browser.js#6623

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I suppose we can hack the isTabEmpty() and consider an about:newtab with the overlay opened NOT empty. That's one more state the content would need to talk to the parent though so I would also suggest we make this P3.

Comment 7

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

Attachment: Bug 1374544 - Open features on onboarding to a new tab

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

Comment 8

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

One of the way to hack it without modifying is to add an empty hash as the patch indicated.

I can't confirm if this interfere what AS want to do though. If they still decide to modify hash we may not use it.