Quicksaver / FindBar-Tweak

A firefox add-on that lets you customize the Find Toolbar the way you like it.
https://addons.mozilla.org/firefox/addon/findbar-tweak/
Mozilla Public License 2.0
91 stars 21 forks source link

Bug on FF51: Findbar doesn't get focus. #268

Closed mzso closed 7 years ago

mzso commented 8 years ago

Hi!

I open the quick findbar as usual (via keyconfig: gFindBar.startFind(gFindBar.FIND_TYPEAHEAD);), but when I type nothing happens because apparently the page still has the focus.

Quicksaver commented 8 years ago

That's strange, what Firefox version are you on?

mzso commented 8 years ago

@Quicksaver commented on 2016. szept. 4. 23:48 CEST:

That's strange, what Firefox version are you on?

As I said in the bug title it's FF51. I also tried the built in hotkeys (alt+num /; shitf+6 on my keyboard). Same thing.

If I press a hotkey twice the findbar does get focuse but the text it had is erased.

mzso commented 8 years ago

Also it only happens when I have findbar tweaks enabled.

Quicksaver commented 8 years ago

Oh sorry, completely missed it! I'll take a look.

mzso commented 8 years ago

I forgot to mention that I'm on Windows (8.1). I'm not experiencing such bugs as the findbar not appearing that OSX users reported. (#263)

By the way did you look into it yet?

mzso commented 8 years ago

Hmmm... There's some breakage on Aurora already, before the new highlighting landed. At least the "highlight all matches by default" option doesn't work for me.

Normally at worst I had to press ctrl+g if the highlights refused to appear, on aurora they only appear if I open the "full" findbar, not for the quick ones.

mzso commented 8 years ago

Hi! The not getting focus thing successfully reached aurora. So I guess it's not tied with the new search highlights after all.

jgjake2 commented 8 years ago

Here is a quick fix for anyone still annoyed with this bug. It's kinda like using duct tape to fix a hole in a boat, but it works for now :/

Edit "fbt@quicksaver/resource/modules/mFinder.jsm" and edit "Modules.LOADMODULE" to look like this:

Modules.LOADMODULE = function() {
    findbar.init('mFinder',
        function(bar) {
            // load our ._remoteFinder now to prevent the original from loading (it still exists and is fully functional, we just don't want it to be used)
            Messenger.loadInBrowser(bar.browser, 'mFinder');

            deinitializeNativeFinder(bar);

            if(bar.browser.isRemoteBrowser) {
                bar.browser._remoteFinder = new RemoteFinder(bar.browser);
            } else {
                bar.browser._finder = new RemoteFinder(bar.browser);
            }

            // if a search is run or re-run and there are no matches, make sure the findbar is visible so this is perfectly clear
            Piggyback.add('mFinder', bar, 'onFindResult', function(data) {
                if(data.result == this.nsITypeAheadFind.FIND_NOTFOUND && data.storeResult && this.hidden) {
                    if(this.open(this.FIND_TYPEAHEAD)) {
                        this._setFindCloseTimeout();
                    }
                    this._updateStatusUI(data.result, data.findBackwards);
                }
            }, Piggyback.MODE_AFTER);

            // this would usually be set when setting bar.browser, but there's no need to do all of that just for registering it with Finder
            bar.browser.finder.addResultListener(bar);

            var origFindbarOpen = bar.browser.finder.onFindbarOpen;
            bar.browser.finder.onFindbarOpen = function(arg){
                if(origFindbarOpen) {
                        return origFindbarOpen(arg);
                }
            };

            var origFindbarClose = bar.browser.finder.onFindbarClose;
            bar.browser.finder.onFindbarClose = function(arg){
                if(origFindbarClose) {
                        return origFindbarClose(arg);
                }
            };

            // get the initial isValid status for this bar's browser
            Messenger.messageBrowser(bar.browser, 'IsValid');
        },

I'm not very motivated right now, but you should if "origFindbarOpen" and "origFindbarClose" are actually functions first and apply all the given arguments instead of just the first one.

Anyway, good luck.

mzso commented 8 years ago

@jgjake2 Thanks for the fix. It works, and that's what matters most.

mzso commented 8 years ago

@jgjake2 The only thing I found to be not working is the automatic highlight all option.

jgjake2 commented 8 years ago

@mzso What version of FF are you using? Because it's working for me. Is there anything in your browser console when you hit Ctrl+F? (Browser console: Ctrl + Shift + J)

jgjake2 commented 8 years ago

@mzso Ok I did find a bug when navigating a site. I guess FBT wasn't getting the memo that the page needed to be re-highlighted. So go to the following files:

Look for a line that looks like: onLocationChange: function(aWebProgress, aRequest, aLocation) { or onLocationChange: function(aWebProgress, aRequest, aLocation, aFlags) {

There should be a line that blocks iFrames from triggering a refresh: if(aWebProgress.isTopLevel) {

remove that line (and the related closing bracket })

This fixed the issue for me. In fact, the feature has never worked better :P

mzso commented 8 years ago

@jgjake2 commented on 2016. szept. 27. 03:10 CEST:

@mzso What version of FF are you using? Because it's working for me. Is there anything in your browser console when you hit Ctrl+F? (Browser console: Ctrl + Shift + J)

Aurora, at present. Also I normally use the quick findbar. Nothing appears in the console.

I'll try your new fix.

mzso commented 8 years ago

@jgjake2 commented on 2016. szept. 27. 04:02 CEST:

@mzso Ok I did find a bug when navigating a site. I guess FBT wasn't getting the memo that the page needed to be re-highlighted. So go to the following files: This fixed the issue for me. In fact, the feature has never worked better :P

This doesn't work for me, if I did it right. Still no highlights by default, or at all with quick findbar. Can you attach the modified xpi just in case I borked the editing?

jgjake2 commented 8 years ago

Aurora, at present

Hmm, I use nightly. I don't know if that would make a difference with this issue though.

This doesn't work for me, if I did it right. Still no highlights by default, or at all with quick findbar. Can you attach the modified xpi just in case I borked the editing?

Sure. Download it here. It just includes the files changed. So just copy/paste them in.

mzso commented 8 years ago

@jgjake2 No joy. Oh, well, I can live with this until Quicksaver updates the addon.

mzso commented 8 years ago

@jgjake2 I just realized the difference might be because you are probably on mozilla central, and I was on aurora. I moved to central today, and the highlights started to function normally.

mzso commented 8 years ago

@Quicksaver Did you loose interest in findbar tweak?

Quicksaver commented 8 years ago

No, I just haven't had much time to work on my add-ons, so I have to prioritize. I should get to FBT still this month. ;)

kilara1988 commented 8 years ago

@jgjake2 Can you please upload the modified XPI file again?The previous link is not working. I tried editing it myself, but FF disabled the addon because it was not verified.

mzso commented 8 years ago

@kilara1988 commented on 2016. nov. 19. 16:42 CET:

@jgjake2 Can you please upload the modified XPI file again?The previous link is not working.

I tried editing it myself, but FF disabled the addon because it was not verified.

Here's the patched xpi:

findbar_tweak-2.1.10-fx-mod2.zip

kilara1988 commented 8 years ago

Thanks,but FF says it can't install it because the file is corrupt.Do i have to use Nightly/Developer Version where the addon signing check is disabled?I am using FF Beta now.

mzso commented 8 years ago

@kilara1988 commented on 2016. nov. 20. 00:24 CET:

Thanks,but FF says it can't install it because the file is corrupt.Do i have to use Nightly/Developer Version where the addon signing check is disabled?I am using FF Beta now.

Of course you need signing disabled, it's not signed. It's not corrupt. Just unpack the archive...

kilara1988 commented 8 years ago

Can it be disabled on FF Beta?I have "xpinstall.signatures.required" set to False,but still FF won't let me install it.And btw i unpacked the archive and tried dragging the XPI file to FF or manually installing it from the Addons Manager,but i get the file corrupted message.I know its because it is not signed and not because it is corrupted.That is why i asked if i need Nightly or i can still disable the check in Beta?

mzso commented 8 years ago

@kilara1988 commented on 2016. nov. 20. 00:34 CET:

Can it be disabled on FF Beta?I have "xpinstall.signatures.required" set to False,but still FF won't let me install it.And btw i unpacked the archive and tried dragging the XPI file to FF or manually installing it from the Addons Manager,but i get the file corrupted message.I know its because it is not signed and not because it is corrupted.That is why i asked if i need Nightly or i can still disable the check in Beta?

I don't remember it claiming to be corrupted. Anyway, you can disable it on every channel but beta. (Unless they stopped being pussies and started enforcing it on stable.)

kilara1988 commented 7 years ago

I am sorry,but how can i install the version which fixes a bunch of bugs from 1 day ago?It doesn't show up on Firefox addons page yet and i can't figure out how to install it from here either.

Quicksaver commented 7 years ago

I will post a beta version for you to try out soon.

Quicksaver commented 7 years ago

All issues should be fixed in beta version 2.1.11b1. Please give it a try and let me know if you still have any problems.

Quicksaver commented 7 years ago

Also, I'm sorry it took me so long to get to this.

talha131 commented 7 years ago

I can confirm, this issue is fixed in beta version 2.1.11b1. Thank you @Quicksaver