1327212 - Findbar doesn't find next match on the page if I open findbar on a new page

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 53, 32bit, ID 20161119030204 (2016-11-19)
STR_1:
1. Open findbar, type "asdf" in findbar
2. Open new tab, navigate to url   data:,asdf
2. Open findbar (Ctrl+F)
4. Click on the "Down" button ("next match") inside findbar

AR:  Step 2 - findbar says "Phrase not found".  Step 4 - no visible action
ER:  Step 4 - findbar should find string on the page, and show the number of matches

This is regression from bug 935521. Regression range:
> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0cb0af1888e39f777862c2c24d84832b6622d40f&tochange=89101092aea13babe5a9a9e4750b89758425f1e2

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1327212 - undo a line removal done in bug 935521 that causes regressions.

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

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8823300 [details]
Bug 1327212 - undo a line removal done in bug 935521 that causes regressions.

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

Comment 3

[:Gijs (he/him)]

Comment on attachment 8823300 [details]
Bug 1327212 - undo a line removal done in bug 935521 that causes regressions.

https://reviewboard.mozilla.org/r/101870/#review102344

::: toolkit/content/widgets/findbar.xml:1351
(Diff revision 2)
>  
>            if (aSelectionString)
>              this._findField.value = aSelectionString;
>  
>            if (aIsInitialSelection) {
> +            this._enableFindButtons(!!this._findField.value);

Why this? If we have no find results, why should we be enabling the buttons based on whether there's text in the field? Shouldn't we just always be disabling them? They'll be enabled once we have find results...

Comment 4

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs from comment #3)
> Why this? If we have no find results, why should we be enabling the buttons
> based on whether there's text in the field? Shouldn't we just always be
> disabling them? They'll be enabled once we have find results...

Well, apparently the expected behavior is that you can start a find-in-page action by clicking the 'next' button as well.
People are just used to hitting buttons, I think, and I do feel that putting this back is making it more consistent compared the current situation as described in bug 1328035.

Comment 5

[:Gijs (he/him)]

Comment on attachment 8823300 [details]
Bug 1327212 - undo a line removal done in bug 935521 that causes regressions.

https://reviewboard.mozilla.org/r/101870/#review102426

Alright, let's try this then.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1ee23936aa8
undo a line removal done in bug 935521 that causes regressions. r=Gijs

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebc1bed3e49f
Backed out changeset e1ee23936aa8 for clipboard tests

Comment 8

[Carsten Book [:Tomcat]]

sorry had to back this out for failing tests like https://treeherder.mozilla.org/logviewer.html#?job_id=66104457&repo=autoland

Comment 9

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=87c0f43d07bb

Comment 10

[Mike de Boer [:mikedeboer]]

Comment on attachment 8823300 [details]
Bug 1327212 - undo a line removal done in bug 935521 that causes regressions.

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3644b91c1e1c
undo a line removal done in bug 935521 that causes regressions. r=Gijs

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3644b91c1e1c

Comment 13

[Paul Silaghi, QA [:pauly]]

Reproduced on Nightly 2017-01-01.
Verified fixed FX 53b1 Win 10, OS X 10.11, Ubuntu 14.04.