Closed Bug 1101670 Opened 5 years ago Closed 5 years ago

UITour: ability to set a search term and show the search popup

Categories

(Firefox :: General, defect)

x86
macOS
defect
Not set
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 36
Iteration:
37.1
Tracking Status
firefox34 + verified
firefox35 + verified
firefox36 + verified

People

(Reporter: benjamin, Assigned: Felipe)

References

Details

Attachments

(1 file)

For the UITour, we want to be able to show the new search dropdown from the UI tour: the best way to do this is to fill in a predefined search term.
Group: mozilla-employee-confidential
We don't clear the search box after performing a search, so some users will have text in the input from their last search... Should the tour restore that text after finishing the tour? [I assume not: I expect it's not useful most of the time (we've have requests to auto-clear it), and it's not like people store their theis there.]

Similarly... Should UITour handle clearing the demo search after closing the tour?
At this point, the answer to both of those questions is "no". We'll overwrite and leave it.
Do we want the first-use UI from bug 1101654 to show as part of this or do we want a way to bypass it in this case?
Flags: needinfo?(benjamin)
It will just show up as normal, no special-casing here.
Flags: needinfo?(benjamin)
Assignee: nobody → felipc
Status: NEW → ASSIGNED
on IRC bsmedberg mentioned that two separate APIs would be better (one for setting the value, one for opening the popup), so I made them like that.
Attachment #8526946 - Flags: review?(benjamin)
Comment on attachment 8526946 [details] [diff] [review]
setSearchTerm and openSearchPanel

I meant to tag Dolske
Attachment #8526946 - Flags: review?(benjamin) → review?(dolske)
Comment on attachment 8526946 [details] [diff] [review]
setSearchTerm and openSearchPanel

Review of attachment 8526946 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/search/content/search.xml
@@ +667,5 @@
>        </method>
>      </implementation>
>      <handlers>
> +      <handler event="input" action="this.inputChanged();"/>
> +      <handler event="drop" action="this.inputChanged();"/>

I think you can avoid these changes / addition of inputChanged() by using searchbar.setUserInput() in UITour instead of setting searchbar.value directly. (See bug 1025483) Assuming the XUL input supports that, if not this is fine. :)

::: browser/modules/UITour.jsm
@@ +519,5 @@
> +        let targetPromise = this.getTarget(window, "search");
> +        targetPromise.then(target => {
> +          let searchbar = target.node;
> +          searchbar.value = data.term;
> +          searchbar.inputChanged();

With a change to setUserInput(), this should probably be paranoid here, and explicitly clear the input when the existing value is the same as the "new" value. (Or else I suspect the various events don't fire? Not 100% sure, or if it matters?)
Attachment #8526946 - Flags: review?(dolske) → review+
Comment on attachment 8526946 [details] [diff] [review]
setSearchTerm and openSearchPanel


You can use ".textbox" instead of "._textbox" everywhere.

Please update https://etherpad.mozilla.org/uitour-ff34 accordingly with the new API info.
Attachment #8526946 - Flags: approval-mozilla-beta+
Comment on attachment 8526946 [details] [diff] [review]
setSearchTerm and openSearchPanel

Review of attachment 8526946 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/modules/UITour.jsm
@@ +526,5 @@
> +      }
> +
> +      case "openSearchPanel": {
> +        let targetPromise = this.getTarget(window, "search");
> +        targetPromise.then(target => {

This API could have been implemented as showMenu("searchPanel") instead of adding a new top-level action. It already supports a callback too.
Flags: qe-verify+
QA Contact: catalin.varga
Points: --- → 2
Flags: firefox-backlog+
Verified as fixed using:

FF 43.RC1
Build Id:20141125180439
OS: Win7 x64, Ubuntu 14.04 x64, Mac Os X 10.9.5

with the demo environment from https://www-demo5.allizom.org/en-US/firefox/34.0/whatsnew/?oldversion=33.1.0
https://hg.mozilla.org/mozilla-central/rev/31c69e3f6181
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Iteration: --- → 37.1
Target Milestone: Firefox 37 → Firefox 36
Verified as fixed using:

FF 35.0b2
Build Id:20141208150535

using https://www.mozilla.org/en-US/firefox/35.0/whatsnew/?oldversion=33.1.0 link
Verified as fixed using:

FF36
Build Id:20141210004006

using https://www.mozilla.org/en-US/firefox/35.0/whatsnew/?oldversion=33.1.0 link.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.