support show single search bar (urlbar) with dropdown displayed with UITour

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Tours
P2
normal
VERIFIED FIXED
3 months ago
2 days ago

People

(Reporter: gasolin, Assigned: rexboy)

Tracking

(Depends on: 1 bug)

unspecified
Firefox 57
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [photon-onboarding])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

(Reporter)

Description

3 months ago
When we use uitour to setHighlight("urlbar") in photon, the cursor is moved to the urlbar, but the dropdown list is not opened as we was in searchbar
(Reporter)

Updated

3 months ago
Flags: qe-verify+
Priority: -- → P2
Whiteboard: [photon-onboarding]
Target Milestone: --- → Firefox 57

Updated

2 months ago
QA Contact: jwilliams
I would recommend implementing this as `showMenu("urlbar")`
(Reporter)

Comment 2

2 months ago
I checked showMenu("search") or showMenu("urlbar") does not work for non-photon/photon cases.

showHighLight("search") does not open the dropdown menu either. The only way is via openSearchPanel()


We could patch openSearchPanel() and open the dropdown for photon universal search.
(In reply to Fred Lin [:gasolin] from comment #2)
> I checked showMenu("search") or showMenu("urlbar") does not work for
> non-photon/photon cases.

I don't understand what you mean?

> showHighLight("search") does not open the dropdown menu either. The only way
> is via openSearchPanel()
> 
> 
> We could patch openSearchPanel() and open the dropdown for photon universal
> search.

FWIW, `openSearchPanel` should have been implemented as showMenu("search"). Maybe we can fix it now.
(Assignee)

Updated

2 months ago
Assignee: nobody → rexboy
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

2 months ago
The dropdown for urlbar can't be opened for a frash-installed user without any search strings because they have no histories to show. If we want to create the onboarding tour for single search on version 57, we may need to make this behavior possible.

I can put some template string on the urlbar before opening, so it can popup as usual to demonstrate the feature, but user then need to erase it and type in what they really need to search. It may be better to figure out if there's a plan ongoing to allow user open the dropdown even under a fresh install and no search strings.
:Mak do you know if we have any plan for handling this fresh-installed user case?
Flags: needinfo?(mak77)

Comment 7

2 months ago
(In reply to KM Lee [:rexboy] from comment #6)
> The dropdown for urlbar can't be opened for a frash-installed user without
> any search strings because they have no histories to show. If we want to
> create the onboarding tour for single search on version 57, we may need to
> make this behavior possible.
> 
> I can put some template string on the urlbar before opening, so it can popup
> as usual to demonstrate the feature, but user then need to erase it and type
> in what they really need to search. It may be better to figure out if
> there's a plan ongoing to allow user open the dropdown even under a fresh
> install and no search strings.
> :Mak do you know if we have any plan for handling this fresh-installed user
> case?

No plan I'm aware of, I didn't know about this need until now.
We may need to brainstorm ideas on what we can do here.

If you put something like "firefox" in the urlbar and it's pre-selected, it should not be a big deal to clear it. We should have that string in the default bookmarks.
Maybe you could even just call URLBarSetURI() when you are done, and that would revert the urlbar to empty?

We may need to triage this, if you could be more specific about what the tour is expecting to show.
Forwarding to Panos for now, so he's aware of this requirement and can make up resources to figure out a strategy.
Flags: needinfo?(mak77) → needinfo?(past)
> If you put something like "firefox" in the urlbar and it's pre-selected, it
> should not be a big deal to clear it. We should have that string in the
> default bookmarks.
> Maybe you could even just call URLBarSetURI() when you are done, and that
> would revert the urlbar to empty?

This is about the unified bar onboarding, right? The above matches my idea about how we should implement this. "Mozilla" and "Firefox" always have matches in the default bookmarks. But I would think that the focus of this tour is to highlight the search functionality, not the history/navigation piece, so I wonder if showing template strings would work best as it would avoid connectivity issues when fetching search suggestions. Do you have UX mockups somewhere?
Flags: needinfo?(past)
(Assignee)

Comment 9

2 months ago
(In reply to Panos Astithas [:past] (please needinfo?) from comment #8)
> This is about the unified bar onboarding, right? The above matches my idea
> about how we should implement this. "Mozilla" and "Firefox" always have
> matches in the default bookmarks. But I would think that the focus of this
> tour is to highlight the search functionality, not the history/navigation
> piece, so I wonder if showing template strings would work best as it would
> avoid connectivity issues when fetching search suggestions. Do you have UX
> mockups somewhere?
Yes, the intension of the tour is to display the search functionality.
Based on the discussions above I'm confirming with UX whether the alternative way you mentioned like keying Mozilla is acceptable, given this is the current way we came up with less engineering work; and we will have some description texts and illustrations on the tour. But if we have a way to emphasize search functionality more, that'd be good.

For now you can refer to mockup of version 56's one click search tour: https://mozilla.invisionapp.com/share/2HB9YE939#/screens/238549395 from bug 1354707.
In version 57 this tour will be replaced by the unified bar tour, with possible text and illustration changes. But the whole design may not change much.
(Assignee)

Comment 10

2 months ago
I'm trying to write a suitable test for opening the urlbar in UITour, but met some question.

1. Calling urlbar.toggleHistoryPopup(); doesn't popup anything, maybe because of the same problem like fresh user?
2. Instead if I set urlbar.value to something, it doesn't popup anything either.
3. Instead if I set a callback to urlbar.popup.onpopupshown and do things like 
    - urlbar.controller.startSearch("fooo");
    or 
    - urlbar.openPopup();
   the listener is called after the popup is out, but urlbar.open is still false so I can't assert it.

I wonder if I shouldn't add listeners directly to urlbar.popup from outside. But I haven't find another way to assert if the popup is out yet.
(Assignee)

Comment 11

2 months ago
Seems if we type in a white space it can pops up with an empty history. This way maybe we can emphasize search feature more. I'll try to do it programmatically and take some video.
(Assignee)

Comment 12

2 months ago
Created attachment 8880310 [details]
Fresh user

Here what I did is popping up the result of entering a whitespace. (then delete it once it popped up so it looks as if nothing entered)
For a fresh user it looks fine and I guess this should be near to what we expected.
(Assignee)

Comment 13

2 months ago
Created attachment 8880313 [details]
A user who has been used the urlbar for a while

And this one is the result of a user who used for a while. The list we get here is
- search with google
- histories

The first "search on google" is actually the result of searching white space, but I think from the screen it don't trouble user at all.
(Assignee)

Comment 14

2 months ago
Hi Verdi,
may you confirm if the screenshot video in comment 12 and 13 can align to what you expected, for the single search tour?
Flags: needinfo?(mverdi)
(Assignee)

Comment 15

2 months ago
(And leave the onboarding overlay alone; I'm just borrowing that to test the URLBar's popup.)
(In reply to KM Lee [:rexboy] from comment #12)
> Here what I did is popping up the result of entering a whitespace.

How is this better than typing "Firefox"?
(Assignee)

Comment 17

2 months ago
(In reply to Marco Bonardo [::mak] from comment #16)
> How is this better than typing "Firefox"?
I talked typing Firefox and Mozilla with Verdi before so the purpose here is adding some possible choices. I think spacing can be a choice because it looks more like what we have with the search bar's tour: clicking it and it expands search bar and get a panel with search engine buttons.
Maybe I can add the result for typing Firefox and Mozilla later for comparison.

Comment 18

2 months ago
(In reply to KM Lee [:rexboy] from comment #14)
> Hi Verdi,
> may you confirm if the screenshot video in comment 12 and 13 can align to
> what you expected, for the single search tour?

That's fine but I do think entering the term "firefox" would be better.
Flags: needinfo?(mverdi)
(Assignee)

Comment 19

2 months ago
Maybe I'll make a patch by using Firefox and send review then. If we decide to use other texts later that'd just be an one-line change.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 22

2 months ago
Those codes of setting search string mainly imitates the code from here:
http://searchfox.org/mozilla-central/source/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js#29

Gijs and Mak may you take a look?

Comment 23

2 months ago
mozreview-review
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review157200

::: browser/components/uitour/UITour.jsm:1413
(Diff revision 4)
> +        let urlbar = target.node;
> +        function urlbarCallback(evt) {
> +          urlbar.popup.removeEventListener("popupshown", urlbarCallback);
> +          aOpenCallback && aOpenCallback(evt);
> +        }
> +        urlbar.popup.addEventListener("popupshown", urlbarCallback);

please pass {once: true} as third argument, instead of manually removeEventListener, and pass the handling function inline to compact code a little bit:

urlbar.popup.addEventListener("popupshown", evt => {
  aOpenCallback && aOpenCallback(evt);
}, { once: true });

::: browser/components/uitour/UITour.jsm:1415
(Diff revision 4)
> +          urlbar.popup.removeEventListener("popupshown", urlbarCallback);
> +          aOpenCallback && aOpenCallback(evt);
> +        }
> +        urlbar.popup.addEventListener("popupshown", urlbarCallback);
> +        urlbar.focus();
> +        urlbar.value = "";

this should be the same string that you pass to startSearch.
And you should also invoke urlbar.select(); after setting value, so the user can clear it more easily.

::: browser/components/uitour/UITour.jsm:1419
(Diff revision 4)
> +        urlbar.focus();
> +        urlbar.value = "";
> +        // Since URLBar doesn't popup history for a fresh new user without
> +        // histories, We trigger a search for "Firefox" to ensure that a fresh
> +        // new user also see the popup.
> +        urlbar.controller.startSearch("Firefox");

I wonder if we should use brandShorterName here, from brand.properties.
Ideally that's the right thing to do in general, I don't know if this should be a special case.

that means we'd search Nightly in Nightly, Firefox in Release/Beta/DevEd.
It should work in general, thanks to default bookmarks.

::: browser/components/uitour/UITour.jsm:1420
(Diff revision 4)
> +        urlbar.value = "";
> +        // Since URLBar doesn't popup history for a fresh new user without
> +        // histories, We trigger a search for "Firefox" to ensure that a fresh
> +        // new user also see the popup.
> +        urlbar.controller.startSearch("Firefox");
> +      });

nit: close the promise chain with a .catch(Cu.reportError)

::: browser/components/uitour/test/browser_showMenu_urlbar.js:8
(Diff revision 4)
> +
> +"use strict";
> +
> +var gTestTab;
> +var gContentAPI;
> +var gContentWindow;

These look unused?

::: browser/components/uitour/test/browser_showMenu_urlbar.js:19
(Diff revision 4)
> +  urlbar.focus();
> +  await showMenuPromise("urlbar");
> +  is(urlbar.popup.state, "open", "Popup was opened");
> +  is(urlbar.controller.searchString, "Firefox", "Search string is Firefox");
> +  urlbar.popup.closePopup();
> +  is(urlbar.popup.state, "closed", "Popup was closed");

I'm not sure how the ui tour harness works, does this happen in a new tab, so that when closed also the text we put in the location bar is gone and won't affect future tests?
Attachment #8879085 - Flags: review?(mak77)

Comment 24

2 months ago
mozreview-review
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review157204

Clearing review given Marco's review. Like Marco, I think the dropdown should just work in a clean profile because of default bookmarks. If automation is the issue and there are no bookmarks / history there so it doesn't work there, create a bookmark in the test instead of modifying the code we ship.
Attachment #8879085 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 25

2 months ago
mozreview-review
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review157672

::: browser/components/uitour/UITour.jsm:1419
(Diff revision 4)
> +        urlbar.focus();
> +        urlbar.value = "";
> +        // Since URLBar doesn't popup history for a fresh new user without
> +        // histories, We trigger a search for "Firefox" to ensure that a fresh
> +        // new user also see the popup.
> +        urlbar.controller.startSearch("Firefox");

It's more preferrable to use Firefox than Nightly per Michael's comment because it brings more related search result by Firefox (whereas Nightly brings something unrelated). I'll ask l10n team's suggestion for that.

::: browser/components/uitour/test/browser_showMenu_urlbar.js:8
(Diff revision 4)
> +
> +"use strict";
> +
> +var gTestTab;
> +var gContentAPI;
> +var gContentWindow;

They are expected by head.js of UITour's test, so if I remove them errors will occur:
http://searchfox.org/mozilla-central/source/browser/components/uitour/test/head.js#4

::: browser/components/uitour/test/browser_showMenu_urlbar.js:19
(Diff revision 4)
> +  urlbar.focus();
> +  await showMenuPromise("urlbar");
> +  is(urlbar.popup.state, "open", "Popup was opened");
> +  is(urlbar.controller.searchString, "Firefox", "Search string is Firefox");
> +  urlbar.popup.closePopup();
> +  is(urlbar.popup.state, "closed", "Popup was closed");

I think so. As far as we use add_UITour_task it should open each task in a new tab (by calling loadUITourTestPage which did that):
http://searchfox.org/mozilla-central/source/browser/components/uitour/test/head.js#436
http://searchfox.org/mozilla-central/source/browser/components/uitour/test/head.js#253
(Assignee)

Comment 26

2 months ago
Let me do the correct reply again. Please ignore comment 25.
(Assignee)

Comment 27

2 months ago
mozreview-review-reply
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review157200

> I wonder if we should use brandShorterName here, from brand.properties.
> Ideally that's the right thing to do in general, I don't know if this should be a special case.
> 
> that means we'd search Nightly in Nightly, Firefox in Release/Beta/DevEd.
> It should work in general, thanks to default bookmarks.

It's more preferrable to use Firefox than Nightly per Michael's comment because it brings more related search result by Firefox (whereas Nightly brings something unrelated). I'll ask l10n team's suggestion for that.

> These look unused?

They are expected by head.js of UITour's test, so if I remove them errors will occur:
http://searchfox.org/mozilla-central/source/browser/components/uitour/test/head.js#4

> I'm not sure how the ui tour harness works, does this happen in a new tab, so that when closed also the text we put in the location bar is gone and won't affect future tests?

I think so. As far as we use add_UITour_task it should open each task in a new tab (by calling loadUITourTestPage which did that):
http://searchfox.org/mozilla-central/source/browser/components/uitour/test/head.js#436
http://searchfox.org/mozilla-central/source/browser/components/uitour/test/head.js#253
(Assignee)

Comment 28

2 months ago
Hi Flod:
We're trying to demonstrate our search bar's feature by typing "Firefox" in the url bar programmatically after user clicked the first-time-use tour. To avoid showing some unrelated search result for a new user, we're now more preferring using "Firefox" across different versions (In contrast with using "Nightly" which gives some unrelated search results in the dropdown). My question is: does it make sense to give the search string "Firefox" hard-coded, so that all users across the world see the search string "Firefox"?
If it's not, should we create a independent entity for that?
Or do you suggest us not do to this and just stick to brandShortName?

Thank you!
Flags: needinfo?(francesco.lodolo)
(Assignee)

Comment 29

2 months ago
Created attachment 8881463 [details]
Typing "Firefox" in searchbar

A quick demo for demonstrating typing Firefox in urlbar.
(In reply to KM Lee [:rexboy] from comment #28)
> Hi Flod:
> We're trying to demonstrate our search bar's feature by typing "Firefox" in
> the url bar programmatically after user clicked the first-time-use tour. To
> avoid showing some unrelated search result for a new user, we're now more
> preferring using "Firefox" across different versions (In contrast with using
> "Nightly" which gives some unrelated search results in the dropdown). My
> question is: does it make sense to give the search string "Firefox"
> hard-coded, so that all users across the world see the search string
> "Firefox"?
> If it's not, should we create a independent entity for that?
> Or do you suggest us not do to this and just stick to brandShortName?


If the final goal is to display "Firefox" and related search+suggestions, I think the safest option is to hard code the search term and don't expose it for localization.

If the term is translated or transliterated, we have very little control over search suggestions that will appear. Note that technically we expose for localization only browser/branding/official (beta, release), Nightly and DevEdition are basically hard-coded.
Flags: needinfo?(francesco.lodolo)
(Assignee)

Comment 31

2 months ago
Thanks for the timely answer Flod!
So based on the discussion above, I think we'll just use Firefox hard-coded. I'll add some comment for why we did this decision on the code so we can trace the reason (and if we changed our mind later).
(Assignee)

Comment 32

2 months ago
mozreview-review-reply
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review157204

Thanks for the comment. It seems that I just didn't get the correct way to trigger search before; seems calling controller.startSearch is the right thing to do for triggering the search, and the dropdown will show as long as the search is started (Even if we got an empty popup at first. So no additional bookmarks for automation is fine.)
Comment hidden (mozreview-request)
(Assignee)

Comment 34

2 months ago
Updated the patch per comments and discussions above.
Mak and Gijs would you help review it again?

Comment 35

2 months ago
mozreview-review
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review157964

::: browser/components/uitour/UITour.jsm:1405
(Diff revision 5)
> +        let brandShortName = Services.strings
> +                             .createBundle("chrome://branding/locale/brand.properties")
> +                             .GetStringFromName("brandShortName");

Leftover?

You still haven't actually explained why this is necessary - the comment still says that the dropdown doesn't work for a fresh user, but that doesn't match my experience. Please clarify.
Attachment #8879085 - Flags: review?(gijskruitbosch+bugs) → review-

Comment 36

2 months ago
(In reply to :Gijs from comment #35)
> Comment on attachment 8879085 [details]
> Bug 1371542 - Support popping up search/history of single search bar
> (urlbar) in UITour.
> 
> https://reviewboard.mozilla.org/r/150420/#review157964
> 
> ::: browser/components/uitour/UITour.jsm:1405
> (Diff revision 5)
> > +        let brandShortName = Services.strings
> > +                             .createBundle("chrome://branding/locale/brand.properties")
> > +                             .GetStringFromName("brandShortName");
> 
> Leftover?
> 
> You still haven't actually explained why this is necessary - the comment
> still says that the dropdown doesn't work for a fresh user, but that doesn't
> match my experience. Please clarify.

To be more precise: I don't understand why the dropdown wouldn't work for a clean profile, especially if, as Marco already said, it has bookmarks. If we show the dropdown icon right now and it doesn't do anything, then we should fix that...
(Assignee)

Comment 37

2 months ago
Created attachment 8881826 [details]
History dropdown for a fresh user

To Gijs:
See the attachment: To be precise, when clicking the history dropdown it just doesn't popup anything for a fresh new user. I thought this is a designated behavior that, when nothing is entered, it just show histories rather than bookmarks or search results; thus nothing is shown for a fresh new user. Let's clarify if this is a bug then.
Flags: needinfo?(gijskruitbosch+bugs)

Comment 38

2 months ago
(In reply to KM Lee [:rexboy] from comment #37)
> Created attachment 8881826 [details]
> History dropdown for a fresh user
> 
> To Gijs:
> See the attachment: To be precise, when clicking the history dropdown it
> just doesn't popup anything for a fresh new user. I thought this is a
> designated behavior that, when nothing is entered, it just show histories
> rather than bookmarks or search results; thus nothing is shown for a fresh
> new user. Let's clarify if this is a bug then.

I think it's a bug to show that dropdown icon and not have it do anything. I could have sworn it used to show bookmarks. Or maybe it should just do that if there's no history at all - if this is also the behaviour people get in permanent private browsing mode that's not great. Marco, do you have background on this?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mak77)
the dropmarker shows typed history, it's likely a fresh user doesn't have any. And I don't think we have code to hide the dropmarker if there's no history (it would be expensive since we'd need to query for that).
note, the behavior is not carved in stone, we could even decide to just show the top frecent pages there, and that would include bookmarks (if they are not disabled in privacy prefs).
There's no data to figure out what'd be better.
Flags: needinfo?(mak77)
(Assignee)

Comment 41

2 months ago
Back to the single search tour case, The main purpose for the tour is to show search feature, so using a search string makes sense to me. If this is unlikely what a showMenu() do to key-in a search string, we can make it a new function in UITour.

But based on comment 38, I think maybe we can do a triage first and then decide whether we can resolve that behavior first.
(Assignee)

Updated

2 months ago
Depends on: 1376911
(Assignee)

Comment 42

2 months ago
Gijs:
Given the solution to the drop marker is not determined yet (and looks more like a feature request to urlbar), do you think we can continue this bug with the solution discussed before, e.g. using Firefox as the search string, or we can have other proposals?
Flags: needinfo?(gijskruitbosch+bugs)

Comment 43

2 months ago
(In reply to KM Lee [:rexboy] from comment #42)
> Gijs:
> Given the solution to the drop marker is not determined yet (and looks more
> like a feature request to urlbar), do you think we can continue this bug
> with the solution discussed before, e.g. using Firefox as the search string,
> or we can have other proposals?

Well, this is targeted for 57, so it seems to me we have time to just fix the url bar instead of working around the issue only to then remove the code again once we fix things 'properly'. Does the onboarding team have capacity to help with that or no?

Note that I'm also not sure how our search deals work, but if this impacts those because now we search for "Firefox" all the time, that's potentially not a desired side-effect...
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs from comment #43)
> Well, this is targeted for 57, so it seems to me we have time to just fix
> the url bar instead of working around the issue only to then remove the code
> again once we fix things 'properly'.

Note that there's no definition of "properly" yet.
I could propose to make the default behavior returning anything, restrict to bookmarks if history is disabled, to history if bookmarks are disabled, and to openPage if both are disabled.
The code is here: http://searchfox.org/mozilla-central/rev/fdb811340ac4e6b93703d72ee99217f3b1d250ac/toolkit/components/places/UnifiedComplete.js#484

But honestly I have no data to tell if the new setting would be better than the old one. For sure "typed" doesn't seem to make a lot of sense, so off-hand I'd be fine with the change.

> Note that I'm also not sure how our search deals work, but if this impacts
> those because now we search for "Firefox" all the time, that's potentially
> not a desired side-effect...

We don't actually "search" for it, we're just fetching suggestions, so I don't think it would be a problem.
(Assignee)

Comment 45

2 months ago
(In reply to Marco Bonardo [::mak] from comment #44)
> (In reply to :Gijs from comment #43)
> > Well, this is targeted for 57, so it seems to me we have time to just fix
> > the url bar instead of working around the issue only to then remove the code
> > again once we fix things 'properly'.
> 
> Note that there's no definition of "properly" yet.
> I could propose to make the default behavior returning anything, restrict to
> bookmarks if history is disabled, to history if bookmarks are disabled, and
> to openPage if both are disabled.
> The code is here:
> http://searchfox.org/mozilla-central/rev/
> fdb811340ac4e6b93703d72ee99217f3b1d250ac/toolkit/components/places/
> UnifiedComplete.js#484
> 
> But honestly I have no data to tell if the new setting would be better than
> the old one. For sure "typed" doesn't seem to make a lot of sense, so
> off-hand I'd be fine with the change.
I asked some UX input about the changing default return of dropdown marker proposal from Stephen. The input is like:
Although he couldn't decide whether changing the default behavior of drop marker to returning everything is better, from his point of view, opening the history dropdown does not actually demonstrate the single search feature very well. What's better is like typing something on the URL bar and make it do actual search.

So from onboarding tour's aspect seems that typing Firefox can now be a better choice. 
For the issue of drop marker, I can loop with UX and decide whether we want to change the behavior by mail or in bug 1376911. It looks like we just need to change the flag a little bit, so maybe I can help on that. We may need some time to discuss about the change though.
Does that sound good?
> > Note that I'm also not sure how our search deals work, but if this impacts
> > those because now we search for "Firefox" all the time, that's potentially
> > not a desired side-effect...
> 
> We don't actually "search" for it, we're just fetching suggestions, so I
> don't think it would be a problem.
Flags: needinfo?(gijskruitbosch+bugs)

Comment 46

2 months ago
(In reply to KM Lee [:rexboy] from comment #45)
> So from onboarding tour's aspect seems that typing Firefox can now be a
> better choice. 
> For the issue of drop marker, I can loop with UX and decide whether we want
> to change the behavior by mail or in bug 1376911. It looks like we just need
> to change the flag a little bit, so maybe I can help on that. We may need
> some time to discuss about the change though.
> Does that sound good?

wfm.
Flags: needinfo?(gijskruitbosch+bugs)

Comment 47

2 months ago
mozreview-review
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review159108

::: browser/components/uitour/UITour.jsm:1415
(Diff revision 5)
> +          aOpenCallback && aOpenCallback(evt);
> +        }, {once: true});
> +        urlbar.focus();
> +        // Since URLBar doesn't popup its dropdown for a fresh new user without
> +        // histories, We trigger it with a search string to ensure a fresh new
> +        // user can see the popup too. To limit the search result as

You may want to change the comment, since we're going to change the empty search behavior, explaining what you just said, that for UX reasons we prefer to search for a string...
Attachment #8879085 - Flags: review?(mak77) → review+

Updated

2 months ago
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Comment 49

2 months ago
Thanks for the review Rik. The comments have been updated.
Gijs, can you take a look again?

Comment 50

2 months ago
mozreview-review
Comment on attachment 8879085 [details]
Bug 1371542 - Support popping up search/history of single search bar (urlbar) in UITour.

https://reviewboard.mozilla.org/r/150420/#review160688

Marco's review is enough here.
Attachment #8879085 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 51

a month ago
okay, thank you guys!
Keywords: checkin-needed

Comment 52

a month ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/89e2d90a7a8a
Support popping up search/history of single search bar (urlbar) in UITour. r=mak
Keywords: checkin-needed

Comment 53

a month ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/89e2d90a7a8a
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
status-firefox56: --- → fixed
Resolution: --- → FIXED

Updated

a month ago
Depends on: 1382700
Hey Rex, how can I access the new 57 tour? Is there a script I can run to test this?
Flags: needinfo?(rexboy)
(Assignee)

Comment 55

14 days ago
Please set browser.onboarding.newtour to:
performance,library,singlesearch,customize,sync
and open a new tab after a few seconds.

For now you still need to turn them on by modifying browser.onboarding.newtour because we haven't collect all illustrations yet before we'll turn them on. this tour corresponds to "singlesearch".
Flags: needinfo?(rexboy)
(Assignee)

Comment 56

14 days ago
(The task for turning 57 tours on is traced in bug 1366056 by the way.)
I have verified this issue is fixed with today's nightly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.