The default bug view has changed. See this FAQ.

Find / FAYT is not focusing on correct tab, so returns no or invalid results

RESOLVED FIXED in seamonkey2.1a3

Status

SeaMonkey
Find In Page
--
major
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: therube, Assigned: mcsmurf)

Tracking

Trunk
seamonkey2.1a3
Bug Flags:
in-testsuite +
blocking-seamonkey2.0.7 -

Firefox Tracking Flags

(blocking-seamonkey2.1 a3+)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; Windows NT 6.1; rv:2.0b3pre) Gecko/20100804 SeaMonkey/2.1a3pre
Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; rv:2.0b3pre) Gecko/20100804 SeaMonkey/2.1a3pre

 
With the new findbar (find toolbar) Find / FAYT is not focusing on the proper page/tab & so returns no, invalid, or unexpected results.
 

Reproducible: Sometimes

Steps to Reproduce:
 
1. Load http://www.seamonkey-project.org/start/
2. FAYT 'download'
> so, / download
> F3 or Ctrl+G to find next instances
> correctly finds (2); 'Download & Releases', 'You've downloaded (or compiled)'

3. Load, in a tab, http://fileforum.betanews.com/browse/new?days=7
4. Return to the tab, http://www.seamonkey-project.org/start/
5. F3 or Ctrl+G
> no action is observed, nothing found

6. FAYT 'download'
> no action is observed, nothing found
 
Actual Results:  
 
Observe the status line.
Repeatedly hit F3.
You will see various http://fileforum.betanews.com/download/* URLs listed
FAYT is finding results from the http://fileforum.betanews.com/browse/new?days=7 webpage
 

Expected Results:  
 
FAYT should find results from the focused page.
 

 
Starting with above scenario, add another tab with the same SeaMonkey page.
So; SeaMonkey | Betanews | SeaMonkey
F3 on this second SeaMonkey page.
> again correctly finds (2) instances of 'download'

Close that page.
F3 to search for another instance, either page will do.

Find returns, "! Phrase not found".
It does this because it is (attempting to) search on the tab that was closed, that is no longer there.
 
At one point, I believe a keystroke, Enter as it was, must have been "transferred" too, as I got a prompt for a download?
 
> Zart: seems like new fayt doesnt hook to tab/windows switches
> Ratty: Ah focus problems

> Ratty: file followup bugs making it block the original SeaMonkey findbar bug

Unfortunately I don't know what the original bug was, & my searching isn't finding it.

Updated

7 years ago
Blocks: 97023

Comment 1

7 years ago
Hah, that sounds as it could be at least related to the problem I've seen that I couldn't get Find/FAYT to work on some rather large text documents.
Interestingly, the "highlight" function of the find bar seems to still work in those cases - same in your scenario?

Updated

7 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

7 years ago
Duplicate of this bug: 583589

Updated

7 years ago
blocking-seamonkey2.1: --- → ?

Updated

7 years ago
blocking-seamonkey2.1: ? → a3+
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → Trunk
Is this bug reporting a regression?
(otherwise, it would look like an issue SM 1.5a already had...)

Comment 4

7 years ago
Serge, it's surely possible that the findbar landing re-uncovered an issue that's been lingering in tabbrowser all the time.
(Assignee)

Comment 5

7 years ago
Created attachment 465777 [details] [diff] [review]
Patch

This seems to do the trick. Got the idea for this from http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/tabbrowser.xml#798
Attachment #465777 - Flags: superreview?(neil)
Attachment #465777 - Flags: review?(neil)

Comment 6

7 years ago
> http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/tabbrowser.xml#798

Wow. This dates back to the Aviary landing. Hg changeset imported from CVS...

Comment 7

7 years ago
Comment on attachment 465777 [details] [diff] [review]
Patch

>+            this._fastFind.setDocShell(this.mCurrentBrowser.docShell);
_fastFind might not be set? (It currently is because the find toolbar causes it to be set, but you never know, we might do it more lazily at some point.)

Comment 8

7 years ago
> _fastFind might not be set? (It currently is because the find toolbar causes it

Using the "fastFind" property (has a getter) instead of "_fastFind" should automatically set and return _fastFind.

> to be set, but you never know, we might do it more lazily at some point.)
As Firefox does ;)
Comment on attachment 465777 [details] [diff] [review]
Patch

Looks good, but as Neil suggests we should be sure to use |this.fastFind| instead of |_fastFind| here [for now].

Firefox at least initiates _fastFind also on addTab. So it can't hurt to be safe on this call.
Attachment #465777 - Flags: superreview?(neil)
Attachment #465777 - Flags: review?(neil)
Attachment #465777 - Flags: review+
(In reply to comment #9)
> Comment on attachment 465777 [details] [diff] [review]
> Patch

p.s. a+=me for landing.
(Assignee)

Comment 11

7 years ago
http://hg.mozilla.org/comm-central/rev/e05dc1486d50
Assignee: nobody → bugzilla
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
I haven't checked myself, but, based on previous comments,
it looks like we may want to check and back-port this to SM 2.0.x.

A test from comment 0 steps would be nice to have...
Flags: in-testsuite?
Flags: blocking-seamonkey2.0.7?
Target Milestone: --- → seamonkey2.1a3

Comment 13

7 years ago
Yes, can we have an automated test for this, please?
2.0.x doesn't use _fastFind.
Flags: blocking-seamonkey2.0.7? → blocking-seamonkey2.0.7-
(Assignee)

Comment 15

7 years ago
Created attachment 466092 [details] [diff] [review]
Idea for a test

This is a test, but it does not work yet. Not sure why, I guess the initialization of FAYT needs to be changed(?) as calling gBrowser.fastFind.find("random", false) once before the first is(....) function makes both test pass.

Comment 16

7 years ago
Does the test in the form where it passes (i.e. with that command added) fail when your patch isn't applied? That should be the most important criteria for the test, after all. :)
(Assignee)

Comment 17

7 years ago
Yes, then it fails. BTW: I tested with alert(gBrowser...) and now it seems the problem is timing-related. When I insert alert("foo") before the is(...) and click on the OK button when the alert box appears, both tests pass with the patch applied. Without the patch the second test fails.

Comment 18

7 years ago
The usual thing we do for such things is observing some event that gets issued when the initialization is done, in doubt adding an observer notification ourselves in that case so we can observe it in the test. Then, we can make the test do it's stuff when the observer is fired.
See e.g. http://mxr.mozilla.org/comm-central/source/suite/common/downloads/tests/browser/browser_nsISuiteDownloadManagerUI.js observing the "download-manager-ui-done" event we added in the download manager window just for that purpose. Notifying observers is cheap but helpful. :)
(Assignee)

Comment 19

7 years ago
Created attachment 466450 [details] [diff] [review]
Another idea for a test

This one does not work either, but I think it's better than the first idea ;-) listens to load event to start the find, but this does seem to be the right event to listen to.
(Assignee)

Comment 20

7 years ago
*this does _not_ seem

Comment 21

7 years ago
Created attachment 467163 [details] [diff] [review]
working test

This test does actually work - and fails without the patch from this bug.
Attachment #466092 - Attachment is obsolete: true
Attachment #466450 - Attachment is obsolete: true
Attachment #467163 - Flags: review?(bugzilla)
(Assignee)

Updated

7 years ago
Attachment #467163 - Flags: review?(bugzilla) → review+

Comment 22

7 years ago
Comment on attachment 467163 [details] [diff] [review]
working test

Pushed this test as http://hg.mozilla.org/comm-central/rev/59416744bb13

Updated

7 years ago
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.