Bug 257061 implements a counter that displays the number of found items AND shows which occurrence is currently selected. This has not been implemented/ enabled for the e10s version (RemoteFinder.jsm). My current assessment is that this shouldn't be hard, mainly adding one or two message passing routes.

Giving to Alex to work on. He should have a patch shortly!

Hi Alexandru, thanks for working on this! I'm afraid you uploaded the wrong patch, because the differences are compared to a state that you used for debugging I think. The view we use to review patches is https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1004515&attachment=8446219 and I need to get all the information from there to see the work that you've done. Can you upload a new patch that correctly diffs between mozilla-central (or inbound) and your patch? If you don't know how to do that exactly, you can ask me anytime on IRC in #fx-team. You can ping me (mikedeboer) or just ask your question(s) directly. I also recommend to use a more descriptive commit message, like `Bug 1004515: add support for the find counter in RemoteFinder.jsm. r=mikedeboer` or something similar.

Comment on attachment 8446219 [details] [diff] [review] Fixed the bug by adding another message for child-to-parent communication. r=felipe As a side note, you don't really need to paste an updated patch in a comment when you upload your next version.

Comment on attachment 8446664 [details] [diff] [review] Bug 1004515 - Patch v2: add support for the find counter in RemoteFinder.jsm. r=mikedeboer Review of attachment 8446664 [details] [diff] [review]: ----------------------------------------------------------------- Next time I'll just r+ this patch, because I couldn't find anything but a few nits. However, just for this one I'd like to see the patch in its entirety with my comments addressed before we land this. Looks like a job well done! Thank you. :) ::: toolkit/modules/RemoteFinder.jsm @@ -1,1 @@ > -// -*- Mode: javascript; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- I don't understand how this ended up on this side of the patch... it's already set to value on the right :/ Please just discard this change. @@ +34,4 @@ > receiveMessage: function (aMessage) { > this._searchString = aMessage.data.searchString; > > + // the parent can receive either one the two the types of messages Nit: We usually write comments as complete, correct English sentences; _with_ an uppercase sentences' first character and punctuation (a dot at the end of each sentence.) @@ +38,5 @@ > for (let l of this._listeners) { > + if (aMessage.name == "Finder:Result") { > + l.onFindResult(aMessage.data); > + } > + if (aMessage.name == "Finder:MatchesResult") { Please make this an `else if` @@ +123,4 @@ > ], > > onFindResult: function (aData) { > + // the parent can receive either one the two the types of messages Nit: same as before, and also the indentation seems wrong here. @@ +127,5 @@ > this._global.sendAsyncMessage("Finder:Result", aData); > }, > > + // receives messages with results of requestMatchesCount > + // and passes them forward to the parent Nit: please correct this comment similarly.

Comment on attachment 8447814 [details] [diff] [review] bug1004515patch_v4.diff Review of attachment 8447814 [details] [diff] [review]: ----------------------------------------------------------------- Oops! This is the wrong diff/ patch. Please post a correct patch that diffs between mozilla-central (or inbound) and your patch. It looks like you made the changes I asked for on top of the work you did before and didn't squash the commits after your work was completed. So in effect, you posted an interdiff here, instead of the full patch. Looking forward to seeing the next version!

Comment on attachment 8448091 [details] [diff] [review] Bug 1004515 - Patch v5: add support for the find counter in RemoteFinder.jsm. r=mikedeboer Review of attachment 8448091 [details] [diff] [review]: ----------------------------------------------------------------- Yes! This works as expected. Thanks Alex!

There was a problem with findAgain in the previous patch. I fixed it in the v6 of the patch.

Comment on attachment 8448440 [details] [diff] [review] Bug 1004515 - Patch v6: add support for the find counter in RemoteFinder.jsm; fixed problem with findAgain. r=mikedeboer Review of attachment 8448440 [details] [diff] [review]: ----------------------------------------------------------------- Ah, good find! However, you did upload a wrong patch... :/

Comment on attachment 8448780 [details] [diff] [review] Bug 1004515 - Patch v6: add support for the find counter in RemoteFinder.jsm; fixed problem with findAgain. r=mikedeboer Review of attachment 8448780 [details] [diff] [review]: ----------------------------------------------------------------- Success! Thanks Alex! ::: toolkit/modules/RemoteFinder.jsm @@ +32,4 @@ > }, > > receiveMessage: function (aMessage) { > + // Only Finder:Result messages have the searchString field. nit: trailing whitespace