Closed Bug 1327212 Opened 3 years ago Closed 3 years ago

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

Categories

(Toolkit :: Find Toolbar, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- verified

People

(Reporter: arni2033, Assigned: mikedeboer)

References

Details

(Keywords: regression)

Attachments

(1 file)

>>>   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
No longer blocks: 1277113
Component: Untriaged → Find Toolbar
Product: Firefox → Toolkit
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
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...
Attachment #8823300 - Flags: review?(gijskruitbosch+bugs)
(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 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.
Attachment #8823300 - Flags: review+
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e1ee23936aa8
undo a line removal done in bug 935521 that causes regressions. r=Gijs
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ebc1bed3e49f
Backed out changeset e1ee23936aa8 for clipboard tests
sorry had to back this out for failing tests like https://treeherder.mozilla.org/logviewer.html#?job_id=66104457&repo=autoland
Flags: needinfo?(mdeboer)
Flags: needinfo?(mdeboer)
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3644b91c1e1c
undo a line removal done in bug 935521 that causes regressions. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/3644b91c1e1c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Reproduced on Nightly 2017-01-01.
Verified fixed FX 53b1 Win 10, OS X 10.11, Ubuntu 14.04.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.