User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1 It has become conventional in Mac programs for command-E to set the find term(s) to the selected text, as if one had hit cmd-C, cmd-F, cmd-V (copy, find, paste). This doesn't work in Firefox, which breaks user expectations. Reproducible: Always Steps to Reproduce: 1. Launch Firefox 2. Select a word or phrase with the mouse 3. Hit command-E Actual Results: Selected text should appear in the Find toolbar as text to be found Expected Results: Nothing happens For comparison, try the same thing in Safari. Although it has no Find toolbar, the selected text is used in a Find (use command-G to find the next instance, or command-F to see it in the dialog).

Sorry, got the expected and actual results revered in the original report.

Pretty sure this is a dupe, but it seems like it could be a good idea beyond mac

The closest I found is bug 250910, which calls for Ctrl+F itself to use the selection by default.

Marking as a dupe of bug 250910. Reporter, if you feel this was done in error, please REOPEN. *** This bug has been marked as a duplicate of 250910 ***

I'd call it related, but not precisely a dupe. The difference is that with accel-E, the text thus entered into the Find field doesn't change if you then deselect it, switch to a different tab, etc.

Cmd+F behaves the way you want: changing the selection doesn't change the text already in the find bar.

No, cmd-F doesn't do what I want. The point isn't to have the selection automatically entered into the find bar when I hit cmd-F; that would be very annoying (to me). The idea is to add cmd-E as a command that puts the current selection into the find bar, as is conventional in Mac UI. (See Safari, Xcode, Stickies -- even

(sigh) -- even third-party apps like Alpha.) Feel free to WONTFIX if there's been a decision that this is a bad idea. From your comment #6, I gathered instead that you thought the bug was irrelevant or already fixed, and that's not the case.

I don't like Safari's behavior: you have to press Cmd+E followed by Cmd+F (or Cmd+G) for it to do anything useful, but the action performed by Cmd+E is invisible, making it hard to discover this sequence of commands. What's wrong with Firefox's behavior? Fwiw, I think we intentionally left Cmd+E unused because it's next to the dangerous Cmd+W on qwerty.

The only thing wrong with Firefox's behavior is that it breaks Mac users' UI expectations. It's a little like not having cmd-X for Cut, or cmd-W for Close. Cmd-E isn't *quite* as ubiquitous as those, but it is part of the platform's UI convention, and it's disconcerting and frustrating to a long-time Mac user when it doesn't work as expected.

Google Chrome

TextMate

Apple Mail

TextWrangler

Xcode

Twitterrific

Terminal

See what I'm getting at here? ;-) Saying “I don't like Safari's behavior” at this point is like saying “I don't like menubars at the top of the screen, I like them as part of the window”. Command-E is a de facto standard on Mac OS X, even if it's not clearly outlined in Apple's docs. Also, even though the action may be considered “invisible” (although it's just as invisible as Command-C/Copy; and the flash of the Edit menu gives a slight indication that something happened), it's not just copying text to a find field. There seems to be a secondary pasteboard that holds (and syncs) the contents of the find field across all apps on Mac OS X. Case in point: Selecting text in TextMate, hitting Command-E, then switching over to Google Chrome and hitting Command-F reveals the same freshly find-pasteboarded text. I use Command-E a lot. The fact that it's missing from Firefox (and that my Mac OS X-trained fingers keep telling Firefox to use a piece of text for find, but Firefox just doesn't behave) has been one of a handful of reasons that have aggravated me enough to stick with Google Chrome and try to coerce it to be more performant and featuresome. Please put this in.

(In reply to Jesse Ruderman from comment #9) > I don't like Safari's behavior: you have to press Cmd+E followed by Cmd+F > (or Cmd+G) for it to do anything useful, but the action performed by Cmd+E > is invisible, making it hard to discover this sequence of commands. What's > wrong with Firefox's behavior? > > Fwiw, I think we intentionally left Cmd+E unused because it's next to the > dangerous Cmd+W on qwerty. This is ubiquitous in OSX apps. It's really disruptive in daily use not to have the behavior that's inconsistent with other OSX apps.

So here's the deal. Not having Cmd-E/F/G behave like expected from Apple's Human Interface Guidelines just made me go back to Chrome. That's the equivalent of not having Cmd-X/C/V. Please just fix this.

(In reply to Martin S. from comment #20) > So here's the deal. Not having Cmd-E/F/G behave like expected from Apple's > Human Interface Guidelines just made me go back to Chrome. That's the > equivalent of not having Cmd-X/C/V. Please just fix this. I recently gave up on Chrome because it got bloated and CPU hungry with the latest Chrome and OS X Mavericks. Before that I was lured away from Safari by Chrome's speed and new features. Now I really want to like Firefox. Really. There's a soft place in my heart for it as the IE killer from back in the day. I dig the philosophy, the foundation, the logo, the name, etc. But between the poor support for multiple user profiles, the bizarre handling of shift+cmd+arrow keys (especially in gmail), the lack of support for cmd+E (this bug) which works consistently in literally every other OS X app I use, and especially the fact that this bug has been open for 8 years, well... I'm with Martin. And it makes me sad. I'll see you in another few years again Firefox, and hopefully we'll be right for each other then.

Even though I don't agree with the sentiments and generally nonconstructive comments in this bug, I do agree we need to have this feature for OSX feature-parity. Blair, what do you think of this patch? Are you ok with reviewing this?

This bit worries me, as I don't think that's what the patch does (if this is what the change in the nsTypeAheadFind.cpp does, then I'm sorry, I'm don't understand C++ beyond what I can logically read from its english): (from Slipp's comment #18) > There seems to be a secondary pasteboard that holds (and syncs) > the contents of the find field across all apps on Mac OS X. Case in point: > Selecting text in TextMate, hitting Command-E, then switching over to Google > Chrome and hitting Command-F reveals the same freshly find-pasteboarded text. I'm not a Mac user, all I ever use it for is to test my add-ons. So I'm not sure how much this actually holds across all apps like that. But if that's true, then the patch shouldn't just set the typeAheadFind searchString to the current browser selection and be done with it (although I would welcome the ability to freely do so). Cmd+E should set this "buffer" of the OS itself, outside of the browser, so other apps (including firefox) can use it too. Maybe there's some sort of API that can be used? There must be something of the sort, if this works for all applications. THEN, Cmd+F/G/F3/whatever would initiate the find bar in the following way: 1) Fill the find bar with the selected text in the browser if there is such, and use that string. (I'm against removing this, as was implied in comment 7; this has been the default behavior forever, and I consider it a feature of the application, rather than a flaw of its OS-integration) 2) If 1) fails (no selection), use the mentioned OS buffer, so pressing Cmd+E on other apps can also place that string in firefox's find field. 3) If 2) fails, open an empty find bar as usual, or use a previously used string in another tab, just like it does now (or should at least, bugs have been filed ;) ).

Hmm, Luís, you're right... CMD-E does persist across apps. Implementing that is way more involved (and also not how I used it before). This is something a platform/ Mac dev should look at, because it'd involve using cocoa APIs. :(

Looking at Firefox 26.0, the cmd-E functionality is now implemented.  I noticed the find bar received a refresh some time in the past few months; I assume the functionality came along with that.  I believe this issue may be closed now as DUPLICATE or WORKSFORME.

I quickly tried Nightly this morning in OS X when I first read the bug, and it most definitely did not work for me.

Regarding the shared find contents, on the Cocoa construct for this is the NSFindPboard and is covered in the Pasteboard Programming Guide (https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/PasteboardGuide106/Introduction/Introduction.html#//apple_ref/doc/uid/TP40008099). Quickly scanning over this, I don't see any specifics on required functionality in situations such as opening find with text selected; AFAIK, it would be fine for Firefox to retain its existing functionality (selection -> NSFindPboard or equivalent). Since the original issue reported is now solved and the remaining issue is a platform-specific cross-API issue, it may be worth opening a new bug to get it to the right people. Cheers.

Not necessary, I almost have a working patch ready that does all this. I was waiting for an excuse to dive more deeply into Cocoa and our C++ widget code ;)

(In reply to Mike de Boer [:mikedeboer] from comment #22) > Created attachment 8371382 [details] [diff] [review] > Patch v1: use cmd-e to set find-in-page search term to currently selected > text > > Even though I don't agree with the sentiments and generally nonconstructive > comments in this bug, I do agree we need to have this feature for OSX > feature-parity. > > Blair, what do you think of this patch? Are you ok with reviewing this? I should point out here that the system search selection works both ways. If I do command-e in app X and then do command-g in Firefox, it should be searching on the search selection from app X.

(In reply to Luís Miguel [:Quicksaver] from comment #26) > I quickly tried Nightly this morning in OS X when I first read the bug, and > it most definitely did not work for me. You know, you're absolutely correct Luís.  I had been mistaking the “selection > findboard” functionality for “cmd-E > findboard” functionality (since the sequence “select-text > cmd-E > cmd-F” shows the find bar with the intended text). @mikedeboer: (In reply to Mike de Boer [:mikedeboer] from comment #28) > Not necessary, I almost have a working patch ready that does all this. I was > waiting for an excuse to dive more deeply into Cocoa and our C++ widget code > ;) If you can make this happen, awesome.  I'm personally on the fence over the selected-text-autofill functionality— on one hand it's traditional to Firefox and relatively unobtrusive to use-cases where a user is simply unaware of it; on the other hand adding shared NSFindPboard functionality makes it potentially obtrusive, and no other browser on OS X does this. This design decision smells a lot like that of `browser.backspace_action`— so perhaps the best option is to create a setting like `browser.selection_find_action` which would be enabled by default (on all all platforms), but configurable to be off (or possibly to other options, like skipping the NSFindPboard sync if the find-bar was populated from a selection).

(In reply to Slipp from comment #30) > so perhaps the best option is to create a setting like > `browser.selection_find_action` which would be enabled by default (on all > all platforms), but configurable to be off (or possibly to other options, > like skipping the NSFindPboard sync if the find-bar was populated from a > selection). "accessibility.typeaheadfind.prefillwithselection", also, my add-on FindBar Tweak has a shortcut checkbox to this in its preferences dialog ;)

Comment on attachment 8372352 [details] [diff] [review] Patch 2.1: save and retrieve search strings to and from the find clipboard Review of attachment 8372352 [details] [diff] [review]: ----------------------------------------------------------------- ::: embedding/components/find/public/nsIWebBrowserFind.idl @@ +47,5 @@ > + > + /** > + * clipboardSearchString > + * > + * The string that will be retrieved from the OS clipboard, if support. It's Nit: supported. @@ +50,5 @@ > + * > + * The string that will be retrieved from the OS clipboard, if support. It's > + * also possible to store a string on the clipboard. > + */ > + attribute AString clipboardSearchString; This is just a utility method right? ::: embedding/components/find/src/nsWebBrowserFind.cpp @@ +247,5 @@ > + > + nsString tmp; > + nsresult rv = GetClipboardSearchString(tmp); > + if (NS_SUCCEEDED(rv) && !tmp.IsEmpty()) { > + *aSearchString = ToNewUnicode(tmp); Shouldn't you modify mSearchString here? @@ +281,5 @@ > + if (NS_SUCCEEDED(rv)) > + return data->GetData(aSearchString); > + } > +#endif > + return NS_ERROR_FAILURE; NS_ERROR_NOT_IMPLEMENTED. @@ +299,5 @@ > + xferString->SetData(aSearchString); > + nsCOMPtr<nsISupports> genericWrapper = do_QueryInterface(xferString); > + > + transferable = do_CreateInstance("@mozilla.org/widget/transferable;1"); > + transferable->Init(nullptr); You need to initialize the transferable with the load context interface from the current document's docshell. @@ +310,5 @@ > + } > + } > + } > +#endif > + return NS_ERROR_FAILURE; NS_ERROR_NOT_IMPLEMENTED. ::: toolkit/components/typeaheadfind/nsITypeAheadFind.idl @@ +55,5 @@ > > > /******************************* Attributes ******************************/ > > + attribute AString searchString; I think this change breaks the way that this interface is currently supposed to work, which is initiate the search operation by calling either find or findAgain.

(In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, emailacopolypse) from comment #39) > This is just a utility method right? Yeah, so I could remove this from the IDL. > I think this change breaks the way that this interface is currently supposed > to work, which is initiate the search operation by calling either find or > findAgain. True, it's meant to be called in the exceptional case when CMD+E is called and the selected string needs to be set as the current search string. An alternative route could be to dismiss this change to `searchString` and KEEP `clipboardSearchString` in the IDL to be called from script, so that the findbar can still manipulate the find clipboard and the next find/ findAgain will pick up the copied string. How does that sound?

oh, and you might want to remove slow responsiveness from your name :P

On second thought, I don't need to touch nsIWebBrowserFind; I'll add a `clipboardSearchString` readonly property and `setClipboardSearchString(searchString, docShell)` to nsITypeAheadFind and direct it from Finder.jsm. That's what the next revision will look like.

(In reply to Mike de Boer [:mikedeboer] from comment #42) > On second thought, I don't need to touch nsIWebBrowserFind; I'll add a > `clipboardSearchString` readonly property and > `setClipboardSearchString(searchString, docShell)` to nsITypeAheadFind and > direct it from Finder.jsm. Here's a question: why do we need any changes in nsIWebBrowserFind or nsITypeAheadFind? Couldn't we just detect the existence of this special clipboard entry in the front-end, and read it out of the clipboard and pass it to the find function instead? (In reply to Mike de Boer [:mikedeboer] from comment #41) > oh, and you might want to remove slow responsiveness from your name :P Haha, well, I have 6300+ unread bugmails at this point, but I prioritize review?/feedback?/needinfo? requests so that's how I saw this on time. :-)

(In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, emailacopolypse) from comment #43) > Here's a question: why do we need any changes in nsIWebBrowserFind or > nsITypeAheadFind? Couldn't we just detect the existence of this special > clipboard entry in the front-end, and read it out of the clipboard and pass > it to the find function instead? Interesting! /me slaps forehead Why didn't I think of this before? On it. > Haha, well, I have 6300+ unread bugmails at this point, but I prioritize > review?/feedback?/needinfo? requests so that's how I saw this on time. :-) Wow, just... wow.

(In reply to comment #44) > (In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, > emailacopolypse) from comment #43) > > Here's a question: why do we need any changes in nsIWebBrowserFind or > > nsITypeAheadFind? Couldn't we just detect the existence of this special > > clipboard entry in the front-end, and read it out of the clipboard and pass > > it to the find function instead? > > Interesting! > /me slaps forehead > Why didn't I think of this before? On it. Thanks, I think that would be much simpler.

sorry for the spam.

I have just a couple of questions if you'll let me. 1) Shouldn't Finder.clipboardSearchString (get) return something, maybe null instead of nothing/undefined, at (line 84 of Finder.jsm in the patch I believe) > if (!kClipboard.supportsFindClipboard()) > return; Same for Finder.prototype.setSearchStringToSelection() method. I guess evaluating the end-result as !string would work the same either way, but thought I should ask, since I keep seeing the "function doesn't always return a value" message in the console when I do this and personally I try to avoid them. 2) line 1060 of findbar.xml in the patch: > if (clipboardSearchString) > initialString = clipboardSearchString; I disagree with this, I think the check should be "if(!initialString && clipboardSearchString)" (comment 23 for reason why), unless you did so knowingly of course; I just think this way it completely nulls the previous prefill case unnecessarily. 3) I was going to suggest removing the aUserInput flag as I thought it was unnecessary, but I see you had the same thought in the meantime. ;)

Luís, thanks for the review!

(In reply to Mike de Boer [:mikedeboer] from comment #52) > Created attachment 8373969 [details] [diff] [review] > Patch 3.4: support the find clipboard in the find toolbar > > Luís, thanks for the review! Since you seemed to like it, here's another one. :) Now, it only uses clipboardSearchString if and only if prefillWithSelection is false. Most users don't even know about this setting (and I don't believe there's any case where gFindBar.prefillWithSelection is false, haven't even seen an add-on that does that), and since it's enabled by default, in practice it will be like the OSX clipboard doesn't exist and none of this will apply. I think instead of checking for the prefillWithSelection preference value itself, it should check as I suggested before, for "!initialString". This way, it checks whether prefillWithSelection made an actual difference to the prefill process.

Grmbl, you're right... again! :)

Comment on attachment 8373434 [details] [diff] [review] Patch 3.3: support the find clipboard in the find toolbar Review of attachment 8373434 [details] [diff] [review]: ----------------------------------------------------------------- Another patch for tests? ::: toolkit/modules/Finder.jsm @@ +19,5 @@ > + "@mozilla.org/widget/clipboard;1", > + "nsIClipboard"); > +XPCOMUtils.defineLazyServiceGetter(this, "kClipboardHelper", > + "@mozilla.org/widget/clipboardhelper;1", > + "nsIClipboardHelper"); Nit: Kill the 'k' prefixes, we've never named XPCOM services as constants. (Except for the cases where we do, but we don't talk about those heretics.) @@ +46,5 @@ > this._listeners = this._listeners.filter(l => l != aListener); > }, > > _notify: function (aSearchString, aResult, aFindBackwards, aDrawOutline) { > + this._searchString = this.clipboardSearchString = aSearchString; This is ambiguous when clipboardSearchString has getter/setter functions, and the getter can return undefined. @@ +65,5 @@ > result: aResult, > findBackwards: aFindBackwards, > linkURL: linkURL, > rect: this._getResultRect(), > + searchString: this._searchString Nit: Unnecessary change? (I consider trailing commas like what was here to be useful) @@ +80,5 @@ > return this._searchString; > }, > > + get clipboardSearchString() { > + if (!kClipboard.supportsFindClipboard()) I wonder if this API should be: Clipboard.supportsClipboardType(nsIClipboard.kFindClipboard) @@ +100,5 @@ > + let dataLen = {}; > + trans.getTransferData("text/unicode", data, dataLen); > + if (data.value) { > + data = data.value.QueryInterface(Ci.nsISupportsString); > + searchString = data.data.substring(0, dataLen.value / 2); /2 isn't safe. But, you don't need it anyway: searchString = data.toString();

Comment on attachment 8373976 [details] [diff] [review] Patch 3.5: support the find clipboard in the find toolbar Review of attachment 8373976 [details] [diff] [review]: ----------------------------------------------------------------- r+ given fixes for comment 55.

(In reply to Blair McBride [:Unfocused] from comment #55) > I wonder if this API should be: > Clipboard.supportsClipboardType(nsIClipboard.kFindClipboard) Well, I think it should be too, but this is the first time we support more than one alternative clipboard. Changing the IDL signature would result in quite a few more changes for Patch 1, because in that case I should remove `supportsSelectionClipboard()` too and propagate that change throughout the code. I'd like to file a follow-up for this, but let's see what Josh thinks, first!

I'll put up a unit test patch when Patch 1 is reviewed as well.

(In reply to Blair McBride [:Unfocused] from comment #55) > /2 isn't safe. But, you don't need it anyway: > searchString = data.toString(); In that case we need to update http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1981 as well :S

Patch with comments addressed, carrying over r=Unfocused.

Comment on attachment 8373348 [details] [diff] [review] Patch 1.2: add find clipboard to the list of available clipboard on OSX Review of attachment 8373348 [details] [diff] [review]: ----------------------------------------------------------------- I'd like to check the fix for the change count bug. Aside from that and the other minor stuff, looks good. Thanks. ::: widget/cocoa/nsClipboard.mm @@ +78,5 @@ > unsigned int outputCount = [pasteboardOutputDict count]; > NSArray* outputKeys = [pasteboardOutputDict allKeys]; > + NSPasteboard* cocoaPasteboard; > + if (aWhichClipboard == kFindClipboard) { > + cocoaPasteboard = [NSPasteboard pasteboardWithName: NSFindPboard]; We do not put spaces between ":" and arguments in Gecko Obj-C code. Please fix the multiple instances of this in this patch. @@ +107,4 @@ > } > } > > + mChangeCount = [cocoaPasteboard changeCount]; This member variable tracked the change count for the general pasteboard. Now you're assigning the change count for one of multiple possible pasteboards. We use this variable to see if we were the last ones to post a change to a board, and that is no longer possible since we don't know which board the count refers to. Please fix this. @@ +375,5 @@ > > +NS_IMETHODIMP > +nsClipboard::SupportsFindClipboard(bool *_retval) > +{ > + NS_ENSURE_ARG_POINTER(_retval); I don't think we're supposed to be using these 'NS_ENSURE*' macros any more. Switch to a replacement or just a normal "if" statement please.

(In reply to Josh Aas (Mozilla Corporation) from comment #61) > @@ +375,5 @@ > > > > +NS_IMETHODIMP > > +nsClipboard::SupportsFindClipboard(bool *_retval) > > +{ > > + NS_ENSURE_ARG_POINTER(_retval); > > I don't think we're supposed to be using these 'NS_ENSURE*' macros any more. > Switch to a replacement or just a normal "if" statement please. Well, I used this because it mirrors its sister function 'SupportsSelectionClipboard' above. Even though this is new code, I do think we should move to a newer/ better style in a 'refactor' pass. The discussion about banning NS_ENSURE_* macros is here: https://groups.google.com/d/msg/mozilla.dev.platform/zLh9fn2eaI8/vUTEvYc6_OQJ - which doesn't appear to state an actionable conclusion.

And Josh, many thanks for the reviewing action!!

And Blair, you too! ;)

Comment on attachment 8374983 [details] [diff] [review] Patch 1.3: add find clipboard to the list of available clipboard on OSX Review of attachment 8374983 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/cocoa/nsClipboard.mm @@ +110,5 @@ > > + if (aWhichClipboard == kFindClipboard) > + mChangeCountFind = [cocoaPasteboard changeCount]; > + else > + mChangeCountGeneral = [cocoaPasteboard changeCount]; All blocks should have {}.

Patch with nit addressed. Carrying over r=joshmoz I pushed the changesets to try: https://tbpl.mozilla.org/?tree=Try&rev=f2e1e2010388 I'll work on a unit test in the meantime.

Added tests for the Find Clipboard and it's interaction with the findbar. Try run: https://tbpl.mozilla.org/?tree=Try&rev=e53b0ef51526

Fixed another test. I will put up a link to a try build once try is unclosed (open) again.

Promised try push: https://tbpl.mozilla.org/?tree=Try&rev=e7071ea263d9

Comment on attachment 8375632 [details] [diff] [review] Patch 6.1: update tests to know about the Find Clipboard on OSX Review of attachment 8375632 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/general/browser_bug537013.js @@ +57,5 @@ > ok(true, "'TabFindInitialized' event properly dispatched!"); > ok(gFindBar.hidden, "Second tab doesn't show find bar!"); > gFindBar.open(); > + let toTest = texts[0]; > + is(gFindBar._findField.value, toTest, Unnecessary change?

Alright! Thanks so much guys, I landed this as: remote: https://hg.mozilla.org/integration/fx-team/rev/3081b9770c33 remote: https://hg.mozilla.org/integration/fx-team/rev/57ceb72568c6 remote: https://hg.mozilla.org/integration/fx-team/rev/c8151192e447 remote: https://hg.mozilla.org/integration/fx-team/rev/f4ab79254dfb remote: https://hg.mozilla.org/integration/fx-team/rev/55fba7bccaaf

backed out due to Linux build failures: https://hg.mozilla.org/integration/fx-team/rev/1b243f066097

Try run with build fixes: https://tbpl.mozilla.org/?tree=Try&rev=0cc0df1beca4

Added `nsClipboard::HasFindClipboard()` implementations that return FALSE on other platforms too. Carrying over r=joshmoz, a=bustage.

Re-pushed as: remote: https://hg.mozilla.org/integration/fx-team/rev/bece94ce2310 remote: https://hg.mozilla.org/integration/fx-team/rev/937f09bd8efc remote: https://hg.mozilla.org/integration/fx-team/rev/2492ef691233 remote: https://hg.mozilla.org/integration/fx-team/rev/4e436323bb5d remote: https://hg.mozilla.org/integration/fx-team/rev/024e6835e6ea

https://hg.mozilla.org/mozilla-central/rev/bece94ce2310 https://hg.mozilla.org/mozilla-central/rev/937f09bd8efc https://hg.mozilla.org/mozilla-central/rev/2492ef691233 https://hg.mozilla.org/mozilla-central/rev/4e436323bb5d https://hg.mozilla.org/mozilla-central/rev/024e6835e6ea

Sorry, backed out in https://hg.mozilla.org/integration/fx-team/rev/a01eebc8baf9 - around one in ten runs on 10.6 debug (which usually means "of the slowest runs"), we're getting https://tbpl.mozilla.org/php/getParsedLog.php?id=34721929&tree=Fx-Team with only one letter of the findbar value filled in.

*sigh* myeah, that's a focusing issue, i.e. the textfield loses focus, then gains focus again and that causes the find field to be populated with the value of the find clipboard. I'm considering to disable that test when the find clipboard is supported - on OSX. More news coming Monday.

Try run with test fixes for 10.6: https://tbpl.mozilla.org/?tree=Try&rev=e232bd7fb96f

Ahum, 'twas the debug build that needed fixin': https://tbpl.mozilla.org/?tree=Try&rev=c0ace382dbb1

Try push with the two failing assertions disabled: https://tbpl.mozilla.org/?tree=Try&rev=d01499445477

Updated patch with test fixes. Carrying over r=Unfocused.

pushed again as: remote: https://hg.mozilla.org/integration/fx-team/rev/dbc693bc48c5 remote: https://hg.mozilla.org/integration/fx-team/rev/becbe15a0b5d remote: https://hg.mozilla.org/integration/fx-team/rev/334ddf5ad1fb remote: https://hg.mozilla.org/integration/fx-team/rev/820d24250606 remote: https://hg.mozilla.org/integration/fx-team/rev/d498b3fa8908

All backed out in https://hg.mozilla.org/integration/fx-team/rev/29273bf136ee for m-oth failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=35003888&tree=Fx-Team

I'm so sorry for the trouble. Try push with definitive fixes for m-bc and m-o tests: https://tbpl.mozilla.org/?tree=Try&rev=a493db44d9fa

After fixing the cause of the last backout and a successful try run, pushed again as: remote: https://hg.mozilla.org/integration/fx-team/rev/0acfd7cfea94 remote: https://hg.mozilla.org/integration/fx-team/rev/ce1491d00b42 remote: https://hg.mozilla.org/integration/fx-team/rev/fe1face47a9c remote: https://hg.mozilla.org/integration/fx-team/rev/819f7d84b703 remote: https://hg.mozilla.org/integration/fx-team/rev/8bd3a004c5f4

https://hg.mozilla.org/mozilla-central/rev/0acfd7cfea94 https://hg.mozilla.org/mozilla-central/rev/ce1491d00b42 https://hg.mozilla.org/mozilla-central/rev/fe1face47a9c https://hg.mozilla.org/mozilla-central/rev/819f7d84b703 https://hg.mozilla.org/mozilla-central/rev/8bd3a004c5f4

I updated the related documentation at https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIClipboard. Sebastian

(In reply to Sebastian Zartner from comment #89) > I updated the related documentation at > https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/ > nsIClipboard. > > Sebastian Thanks Sebastian! I made a small update.

(In reply to Blair McBride from comment #55) > (From update of attachment 8373434 [details] [diff] [review]) > > + searchString = data.data.substring(0, dataLen.value / 2); > > /2 isn't safe. But, you don't need it anyway: > searchString = data.toString(); FYI toString() is horribly inefficient here; nsSupportsString has to allocate a copy of its internal nsString, which JS then has to make another copy of, whereas .data simply makes another reference to the string and this reference is passed through to JS as an external string without any copying (and if JS then passes the exact string to an AString or DOMString then that avoids yet another copy).