Closed Bug 567306 Opened 14 years ago Closed 13 years ago

Find command (Ctrl+F) fails to pick up selected text from the page

Categories

(Toolkit :: Find Toolbar, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- final+

People

(Reporter: alice0775, Assigned: asaf)

References

Details

(Keywords: regression, Whiteboard: [has patch][hardblocker])

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100520 Minefield/3.7a5pre ID:20100520065946
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100520 Minefield/3.7a5pre ID:20100520065946

When I use "Find"(ctrl+F) for the first time after the start of the browser window, "Find" (ctrl+F) does not start looking for it with a selected text on the actual page.
And the selected text should be copied to the FindBar.

However, "Find" after the second times does not have any problem.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open HOME( http://www.mozilla.org/projects/minefield/ )
3. Select text on the page
4. Ctrl + F

Actual Results:
 The selected text is not  copied to the FindBar.
 "Find"(ctrl+F) does not  start looking for it with a selected text on the actual page.


Expected Results:
 The selected text should be copied to the FindBar.
 And "Find"command should start with the selected text.



Regression window:

Works:
http://hg.mozilla.org/mozilla-central/rev/d5d5ed6d3e1c
Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100518 Minefield/3.7a5pre ID:20100519083331

Fails:
http://hg.mozilla.org/mozilla-central/rev/046c2d2acd3b
Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100518 Minefield/3.7a5pre ID:20100519101258

Push log:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d5d5ed6d3e1c&tochange=046c2d2acd3b

Candidate:
Bug 566736  - Lazily initialize the find toolbar
blocking2.0: --- → ?
So there's a line of code in the findbar constructor aSelf.browser = aSelf.browser to add the keypress listeners and so on. It runs in a timeout, in case the browser hasn't yet been bound when the findbar is constructed.

The browser setter touches the textbox value, so with lazy initialization we get the following sequence:
- Constructor runs, schedules the timeout code
- startFind looks for a selection, finds it, sets the value of the textbox
- timeout code runs, blows away the selection text we just found

What we *really* need is Bug 202563. Then the widget could just deal, either adding the handlers straight away if document.readyState == complete or in an onLoad handler.

The hack fix is I guess for the browser's command handler to check gFindBarInitialized, and queue startFind to run in a timeout. Of course, then all the "this" references in the findbar are hosed...

Or we make all users of the findbar widget responsible for ensuring the browser has been bound before the findbar is constructed.
blocking2.0: ? → final+
Blocks: 97023
Attached patch patch with test (obsolete) — Splinter Review
Assignee: nobody → mano
Status: NEW → ASSIGNED
Attachment #449343 - Flags: review?(gavin.sharp)
The |let Ci = ... | line will be removed on check-in.
Comment on attachment 449343 [details] [diff] [review]
patch with test

>diff -r 979fbcd8bb29 browser/base/content/test/browser_bug567306.js

>+function onFocus() {
>+  ok(!gFindBarInitialized, "find bar is not yet initialized");

This will fail when run after browser_typeAheadFind.js - should probably open a new window and test in that (for both tests, even). r=me with that fixed.
Attachment #449343 - Flags: review?(gavin.sharp) → review+
Attached patch fix both tests (obsolete) — Splinter Review
Attachment #449343 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Callek is backing this out for me. Details coming.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Thought it failed three ways, but the third was the same as the first:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1275698197.1275699181.10045.gz
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | Found an unexpected browser window at the end of test run
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | Found an unexpected browser window at the end of test run
TEST-PASS | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | before wait for focus -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object XPCNativeWrapper [object Window]]) about:blank desired window: ([object ChromeWindow]) chrome://browser/content/browser.xul child window: ([object XPCNativeWrapper [object Window]]) about:blank docshell visible: true
TEST-PASS | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | already focused
TEST-PASS | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | maybe run tests <load:true, focus:true> -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object XPCNativeWrapper [object Window]]) about:blank desired window: ([object ChromeWindow]) chrome://browser/content/browser.xul child window: ([object XPCNativeWrapper [object Window]]) about:blank docshell visible: true
Running chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js...
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js :: test :: line 53"  data: no]
TEST-INFO | checking window state
TEST-PASS | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js | before wait for focus -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object XPCNativeWrapper [object Window]]) about:blank desired window: ([object ChromeWindow]) chrome://browser/content/browser.xul child window: ([object XPCNativeWrapper [object Window]]) about:blank docshell visible: true

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1275698198.1275699245.10286.gz
TEST-INFO | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | Console message:
*** registerProtocolHandler(testprotocol,https://example.com/foobar?uri=%s,Test Protocol)
TEST-PASS | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | Notification box should be displaying outside of private browsing mode
TEST-UNEXPECTED-FAIL | automation.py | application timed out after 330 seconds with no output

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1275701256.1275702230.22982.gz
(same as the first)
(In reply to comment #7)
> Callek is backing this out for me. Details coming.

Done: http://hg.mozilla.org/mozilla-central/rev/28086cf6ede8
Windows:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1275703018.1275704180.31812.gz
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js :: test :: line 53"  data: no]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js | Found an unexpected browser window at the end of test run
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_searchbar.js | Found an unexpected browser window at the end of test run
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_sslsite_transition.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_theming.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_theming.js :: test :: line 52"  data: no]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_transition.js | There should only be one tab - Got 2, expected 1
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_transition.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_transition.js :: test :: line 77"  data: no]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_ui.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js :: PBUI_toggleMode :: line 10160"  data: no]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_urlbarfocus.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_urlbarundo.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_urlbarundo.js :: test :: line 47"  data: no]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_viewsource.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js :: testTabTitle :: line 79"  data: no]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_zoom.js | Exception thrown - [Exception... "'Failure' when calling method: [nsIPrivateBrowsingService::privateBrowsingEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_zoom.js :: test :: line 48"  data: no]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_viewsource.js | Entering the private browsing mode should not open any view source window
TEST-UNEXPECTED-FAIL | automation.py | application timed out after 330 seconds with no output

http://tinderbox.mozilla.org/Firefox/1275701136.1275703247.27638.gz is the link to download the full Windows debug log, which is the only way I could find to load it, since browser_privatebrowsing_protocolhandler.js went into a loop of assertions about XPConnect is being called on a scope without a 'Components' property! that wound up overrunning the output buffer ("Output exceeded 52428800 bytes, remaining output has been truncated" after a few trillion repeats).
Comment on attachment 449388 [details] [diff] [review]
fix both tests

>+/* Any copyright is dedicated to the Public Domain.
>+ * http://creativecommons.org/publicdomain/zero/1.0/
>+ */

Is this intentional?
Yes.
Blocks: 569342
Is there an ETA on finishing this? It currently blocks me landing bug 569342 because of the test issues.
Any chance this is fixed soon ?
(In reply to comment #13)
> Is there an ETA on finishing this? It currently blocks me landing bug 569342
> because of the test issues.

Mano, what remains to finally get those fixes and the new test checked-in?
Status: REOPENED → ASSIGNED
Whiteboard: [has reviewed patch]
Any change, status, eta?
Whiteboard: [has reviewed patch] → [can land]
I should have noted this before: the patch is ready and fine, but the tests are failing in a way that I cannot reproduce locally (the failure is related to the way we run tests, not to the bug itself).
Grrr. I just relanded this because it's got [can land] in the whiteboard.

http://hg.mozilla.org/mozilla-central/rev/9fd4170ac591

Mano, are you going to look into the test failures?

/me goes to back out
Whiteboard: [can land] → [test failures][needs new patch]
can someone take over from Mano, he appears to be too busy with other things.
I'll revisit this again this week.
Whiteboard: [test failures][needs new patch] → [test failures][needs new patch][eta 26/11]
Summary: Find command(ctrl+F) does not start looking for it with a selected text on the actual page → Find command (Ctrl+F) fails to pick up selected text from the page
Whiteboard: [test failures][needs new patch][eta 26/11] → [test failures][needs new patch][eta 26/11][soft blocker]
Whiteboard: [test failures][needs new patch][eta 26/11][soft blocker] → [test failures][needs new patch][eta 26/11][softblocker]
Whiteboard: [test failures][needs new patch][eta 26/11][softblocker] → [test failures][needs new patch][eta 15/1][softblocker]
Ping.
Ping.
I'm not really sure about this , but on a slow computer it appears that the text does show in the find box and immediately disappears again.
Or at least a blue "flash" is visable.
Ping?
Pinging in a bug really is unhelpful and just creates a lot of noise. Please don't do it.
Whiteboard: [test failures][needs new patch][eta 15/1][softblocker] → [test failures][needs new patch][softblocker]
(In reply to comment #25)
> I'm not really sure about this , but on a slow computer it appears that the
> text does show in the find box and immediately disappears again.
> Or at least a blue "flash" is visable.

Yes, I did some tests keeping my computer busy while trying ctrl+F after startup and indeed the text is copied into the box first time, the box turns blue and than the text and color disappear.
This should have never been left open for 9 month.
Basic functionality that doesn't work right away but needs a second try is a joke.
Requesting [hard]blocking FF4.0
blocking2.0: final+ → ?
Whiteboard: [test failures][needs new patch][softblocker] → [test failures][needs new patch]
I think I agree - regressing a longstanding primary UI (kinda) feature so that it eats user text? Sounds hardblocking to me.
blocking2.0: ? → final+
Whiteboard: [test failures][needs new patch] → [test failures][needs new patch][hardblocker]
Rebasing this patch.
Attached patch Proposed patch.Splinter Review
Here's a rebased patch. Tests pass for me locally on Mac, but I'll run this on try to be sure.
Attachment #449388 - Attachment is obsolete: true
Whiteboard: [test failures][needs new patch][hardblocker] → [test failures?][has patch][hardblocker]
OS: Windows 7 → All
Hardware: x86 → All
I've also run privatebrowsing b-c tests with this patch and I've not seen any failure. Please keep us updated on try results, we can cooperate to figure out what's up. Due to how pb tests work it's possible we are hitting a case where stuff stays alive due to a BrowserShutdown call before DelayedStartup, while looking at this I made the patch in bug 631447 that removed most of the assertions from these tests.
Since Patrick experiments were largely positive I've pushed to central

http://hg.mozilla.org/mozilla-central/rev/9ca5948baa2d
Status: ASSIGNED → RESOLVED
Closed: 14 years ago13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [test failures?][has patch][hardblocker] → [has patch][hardblocker]
Target Milestone: --- → mozilla2.0b12
Mozilla/5.0 (Windows NT 5.1; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre ID:20110216030352

VERIFIED

thanks
Status: RESOLVED → VERIFIED
Depends on: 869832
You need to log in before you can comment on or make changes to this bug.