feinstaub / webextension_local_filesystem_links

"Local Filesystem Links" is an addon for the Mozilla Firefox webbrowser. It will be ported soon to the WebExtension API and thus also might work for Chromium
https://addons.mozilla.org/en-US/firefox/addon/local-filesystem-links/
GNU General Public License v2.0
51 stars 27 forks source link

Multiple windows opening #125

Closed snugga closed 1 year ago

snugga commented 6 years ago

This has only started happening since updating to the most recent version.

In proworkflow when I click a file link it now opens up in Microsoft Explorer three times instead of just the once.

Has there been a change to how the file paths should be done?

AWolf81 commented 6 years ago

No, file paths are unchanged.

This behavior is improved in latest version but maybe I've missed something.

Maybe I need to have a look at proworkflow and check it there - I don't know it. But I could test it with the trial version.

Where can I add the file links in proworkflow?

AWolf81 commented 6 years ago

A comment in the addon gallery mentions that this issue is still present (opens 10 times with one click). So I tried to reproduce the issue and the only thing how I could get the mentioned behaviour was by dragging a tab with file links to a new window and then clicking on a link and it opens the file three times. (The mentioned 10 times is probably happening if the link is clicked several times - it's easily possible to have 10 instances open.)

Not sure if this is the same issue in Proworkflow but it's probably related. I have to check the code & do some debugging to see what happens here.

ITaluone commented 5 years ago

This has only started happening since updating to the most recent version.

In proworkflow when I click a file link it now opens up in Microsoft Explorer three times instead of just the once.

Has there been a change to how the file paths should be done?

I agree. Same symtoms

snq commented 5 years ago

This issue is still present in version 99.62. I made sure I don't click on the link several times. A single click causes at least 5-6 Explorer Windows to open.

CopaceticMeatbag commented 4 years ago

Just adding a note to say I am experiencing this too; I have several clients who use this regularly and they'll often complain that clicking to open a local link results in many copies of the program for the link being opened, usually ~10-15 copies. I've had to make a quick "taskkill" batch script for them to run as a terrible workaround for the moment so they don't have to manually close them all.

I have found that the issue will continue to reproduce itself until the web page is refreshed, at which point opening links functions normally again.

AWolf81 commented 4 years ago

Thanks for reporting.

What instances are running? Is it Explorer.exe? If it is, there is a fix by updating the host app as mentioned in #145. So I have to update, test and release the app for Windows.

Your clients are using Windows, right?

I'll try to create the host app later today. Last time, I had issues with the installer creation.

CopaceticMeatbag commented 4 years ago

Yes, all are using Windows 7. Re instances, sometimes explorer.exe but most notably happens with MS Word (latest 365 build) opening .doc/.docx file links. It seems like it's left to explorer to delegate opening the links though, so the fix could still be relevant? Are you able to recompile with that change added please?

CopaceticMeatbag commented 4 years ago

I didn't attempt recreating the installer, however recompiled the host app with the changes specified in #145 . Uploading here in case it's of use to anyone else - still testing here, but so far so good.

edit: Welp, it's still broken :( something else is causing the multiple opens. For now I've setup a workaround for the extension to trigger an autohotkey app that handles launching instead.

CopaceticMeatbag commented 4 years ago

Just an update - I rewrote a simplified host app and tried a few different launch methods, found out the issue is definitely in the extension side sending multiple requests to the host for the same file, not an issue on the host app.

AWolf81 commented 4 years ago

Thanks @CopaceticMeatbag for your tests. I've also tried to change the host app as mentioned in the other issue but also with-out success.

I'll check the sending side and try to spot the location that is sending multiple messages to the host app.

How does the workaround work? Sounds interesting. What's the name of the hotkey app? How is the local filesystem extension triggering the hotkey app?

CopaceticMeatbag commented 4 years ago

A colleague of mine finally figured this one out:

"I suspect it's caused by duplicate event handlers getting attached to the page probably after ajax page loads. So I’ve added an underscore _debounce wrapper around the file open function that will hopefully stop it being triggered multiple times."

jid1-JAzC7z53jemo5Q@jetpack.zip

hartmann-daniel commented 4 years ago

Hi there. I have the same problem with opening Explorer several times. Sometimes the Explorer doesn't open at all. The icon is also temporarily not displayed. The software in which the extension is used has a lot of Ajax requests. So fits your diagnoses.

I would be happy if your solution worked and a current version was provided.

Kind regards Daniel

em99 commented 4 years ago

Hi I have the same problem with firefox 73.0.1. on win10. Sometimes it opens up 10 windows! please, can you fix it?

AWolf81 commented 4 years ago

I think this will be fixed once the PR #159 is merged. The problem was that the content script was added multiple times to a tab.

I've improved the tracking of the injected scripts in the background script and only inject if there is no content script connection available.

Also dynamic links are back by using MutationObserver. So ajax enhanced/modified pages should work. A debounce of the onUpdate handler in the background script was required too - with-out it I had multiple events triggered in Trello.

em99 commented 4 years ago

is the new version available?

AWolf81 commented 4 years ago

@em99 it should be fixed with version 0.99.64. It's available in the Mozilla store. Please let me know if the issue is still present.

em99 commented 4 years ago

thank you @AWolf81 , I'll check as soon as I'll be back at work!

pandagurlie commented 4 years ago

I have version 0.100.2 and this is still happening for us. If I restart Firefox it resets it and folders only open once, but my coworker says that doesn't work and it opens 3 folders for her every time.

AWolf81 commented 4 years ago

@pandagurlie is this happening in Salesforce?

OK, I'll check if I can improve it.

CopaceticMeatbag commented 4 years ago

I can confirm it's happneing for me as well (just got a report this morning), seems to be only Windows Explorer links and nothing else.

pandagurlie commented 4 years ago

Yes, we are using this in Salesforce. We are only opening Windows Explorer folders.

em99 commented 4 years ago

version 0.100.2 and this is still happening using Firefox 76.0

hollanb commented 4 years ago

this is happening again now, when there is more than one link to the file it creates the folder link after the lkinbk, if i hide that it still opens up is there a class i can add to only add exploerer folder link to a href?

Jaroost commented 4 years ago

I solve this by adding a additional / to the file link For the following UNC path : \\server\folder the file link must be : file://///server/folder

The bug appeared sometimes (after generaly 1 refresh) when file link was: file:////server/folder Ok forget what I said it opens 3 windows sometimes and I don't know why

em99 commented 3 years ago

Hi @AWolf81 is there any chances of getting this fixed?

AWolf81 commented 3 years ago

Hi, if I have a reproduction of the issue I can try to fix it.

I'm not using Salesforce, is it free to use? A html page that is showing the effect would also help.

With-out that, it's difficult to improve the script. I know that the event handling is not perfect and there are multiple events in some cases (probably caused by async. page creation with JavaScript).

Maybe longer debounce time could help but I want to keep it as short as possible (affects user experience).

em99 commented 3 years ago

I don't know how to help you, Sometimes it opens one folder, sometimes 3 or more.... looks random to me.

I don't use Salesforce, just on my local server, "file://///192.168.1.102/path...../"

AWolf81 commented 3 years ago

can you please post a link to the code that will have that behavior?

Or can you create a CodeSandbox with a link that will have the problem?

Or if it's really random and not specific to a page, can you please try if drag a tab to a new window is causing the problem. I'm not sure if that is still a problem.

em99 commented 3 years ago

so sorry @AWolf81 , i'm just an user, can't post you any link, don't know what a CodeSandox is.

just updated firefox to 83.0 and it opens a lot more windows then before.

AWolf81 commented 3 years ago

@em99 no problem. I'll have a look why it is getting worse with the new FF version.

Jaroost commented 3 years ago

Hello here a link that sometimes open 3 explorer windows sometimes only 1:

<a class="btn btn-primary" type="button" target="_top" data-toggle="tooltip" data-placement="bottom" title="" style="height:34px;padding: 5px 5px 1px 5px;min-width:40px;" href="file://///FS41/DATA1/Technique/Backup-Machines/backup_files/283.000.00/283.964.00/keb_converter/2020.12.03_14.41.44" data-original-title="Ouvrir ce backup">
<span class="far fa-folder-open" aria-hidden="true" style="font-size: 20px;"></span>
<span style="vertical-align: top;"> </span></a>

To reproduce the bug, here what I'have done:

Note that this procedure doesn't work all the time

agri17 commented 3 years ago

Hi there, we are testing v0.100.2 with Confluence DC v7.4.3, Firefox v85.0. Word .docx and .png open four windows, whereas .pdf only opens one (Foxit reader). Regards

em99 commented 2 years ago

Hi @AWolf81 is there any chance that you could fix this issue? I'm using this everyday! i really need it, I am willing to pay of course!

CopaceticMeatbag commented 2 years ago

@em99 I actually ended up fixing this one, will be able to post the code on monday :)

em99 commented 2 years ago

Hi @AWolf81 is there any chance that you could fix this issue?

AWolf81 commented 2 years ago

@em99 sorry, no progress, on this issue.

I could trigger the behavior at the test webserver at the dynamic links. Fast clicking + switching tabs with dynamic links but I still don't know what's causing it.

At the moment, I can't debug the extension because the dev environment is not starting (I haven't worked for some time on the extension). I'm getting an error message like Temporary add-on installation is not supported in this version of Firefox (you need Firefox 49 or higher) - I have FF 96.0.1 installed (so not sure what is causing this).

So maybe I can find some time next weekend but I can't promise anything. I first have to get the development environment fixed then I can do some tests.

@CopaceticMeatbag mentioned some fix but not sure if he could fix it.

casiosmu commented 2 years ago

I have a similar issue: The links in your demo fiddle work fine. But the link in our internal app opens up 6! explorer windows. It happens if I click the link itself and also the icon appended by this extension.One difference to the fiddle I can see: the app uses AngularJS and the ng-href directive additionally to the href. The link has the format: <a target="_blank" ng-href="file:\\\\\servername\folder1\...\folderX" href="file:\\\\\servername\folder1\...\folderX"><span class="glyphicon glyphicon-new-window"></span></a>

e3elettronica commented 1 year ago

We are having the same issue with version 0.100.2 and Firefox 102.10.0esr (32bit). We are experiencing the issue in Jira Service Management Cloud / Jira Work Management Cloud pages. When we click on a directory link it opens 3 times Windows Explorer. When doing the same on a file link sometimes it does not occur, probably because the triggered application does not allow to open different instances of the same file (eg. Acrobat Reader or Excel). Is there any working workaround applicable? Or any simplified version of the plugin / native app which, even if losing some functions, would do the job. Eg. if the issue is triggered by multiple retries on failure / timeout, for us would be good enough to fail with no error message as long as it opens the links properly only once.

hartmann-daniel commented 1 year ago

hello everyone

I too would like to inquire whether there is a change to fix the error in the final. I would like to use the addon again because it would make my everyday life a lot easier. Please check how much effort it will cost you to start the addon again without the problems. Maybe we can raise enough donations to offset your effort.

Best regards Daniel

CopaceticMeatbag commented 1 year ago

I finally found the fix we did, which was to extend the clipboardGetFilePath extension to allow sending paths to the native app. You'll need to adjust the .reg file to point to the json location of the app, and update the background.js file to specify the URLs it should work for (we just filtered by our domain on line 4). FirefoxAddin.zip

AWolf81 commented 1 year ago

I could fix my issue with the development environment - upgrading the web-ext dependency to 7.6.2 fixed it. So npm start is working now.

Not sure about the zip file above. That's not the extension of this repository and it's using another unknown native app. I also don't understand why to use the clipboard.

To the issue I've debugged it today, and it looks like the content script runs twice on each page load or tab switch. That's causing to have multiple event handlers attached. It seems not related to the dynamic link handling, so just the page load causes the problem. I could easily get 6 Explorer instances by opening two tabs and switching between them. Especially a reload of one tab and switching to the next causes the issue.

Current status of testing - FF 113.0.1 (64-Bit) - master I haven't figured out why the activate function is running at least twice. A workaround that could work is to run unregisterEvents() before registerEvents() here.

With this, it seems to work but I'd like to avoid this. Would be better to find the cause of the second run or to have a check that's showing that the events are already attached.

maxm199 commented 1 year ago

@AWolf81 tried today to make the changes you suggested, putting unregisterEvents() before registerEvents() in content.js then compiled but still I can open multiple windows if I open multiple tabs and refresh one then go to another and then click the link. It seems it's one for each tab I refresh, if I refresh only one new tab then it open 2 windows, if I refresh 2 new tabs then it opens 3 windows... but I'm just speculating.

AWolf81 commented 1 year ago

@maxm199 thanks for your test. I'll test this again tomorrow.

But I think this should work. Because this will allow to re-run activate function multiple times for a tab without having the issue.

Not sure, if this has an performance impact.

I also tried to check if the events are already attached but that wasn't working. Tried with $._data($(document).find(fileLinkSelectors.join(', ')).get(0), "events").

During my tests it was always undefined. But I think I'll try this again and only call registerEvents if there are no events.

maxm199 commented 1 year ago

@AWolf81 I just did the change you suggested, nothing else, so if there was anything else to do to make it work I did not made it. Maybe multiple times for a tab is not the issue here, within my tests in the refreshed tab (even multiple times refreshed) I did not have any multiple windows opening, I always had them going back and clicking on the not refreshed tab.

AWolf81 commented 1 year ago

@maxm199 I think you're right, the unregister & register approach is also not working for me. It just looked like it is working because the duplicated events are not happening at every reload or tab switch.

Checking the events is not working in the content script. Tried with $._data(document, "events") but it seems that it would be attached even with that check before calling registerEvents.

At the moment, I'm checking the background script that is handling the injectedTabs array. I found one spot that looks promising but I have to do more tests. In extension\background\index.js I replaced finally with then in line 56 here

I have to check the cases where this block is used. Removing the then completely then new pages won't have the file links at all.

It would be also interesting to see the response from the content script as the response to the ping. Not sure, why it is not there.

Also updating the extension settings needs to be tested with this change. E.g. are the settings correctly applied on a whitelist change?

CopaceticMeatbag commented 1 year ago

@AWolf81 the clipboard was our workaround to pass the desired path to the native app to open (which just passes the received string to the default system open handler). Just thought I'd post as an alternative approach and what we ended up using. Not sure if there's any useful clues in the fact that using clipboard doesn't generate multiple windows, maybe it implies the issue isn't in the event trigger?

AWolf81 commented 1 year ago

@CopaceticMeatbag ah, OK, thanks. I think the native app is not causing the issue here.

I think the problem we're having here is that the background script is managing the tabs in which we're injecting the content script and this handling is not working properly. This handling is needed for the whitelist feature to only inject in whitelisted tabs.

At the moment, I'm working on this and I think I have to add the injectedTabs into local storage so a refresh or a restart of the background script is not losing the information about the injected tabs.

Snippet from my background/index.js constructor:

    this.injectedTabs = {};
    browser.storage.local.get('injectedTabs').then(injectedTabs => {
            console.log("loaded tabs from local storage", injectedTabs);
            this.injectedTabs = injectedTabs;
        })

Changed data from injectedTabs = [ 1, 2 ] to injectedTabs = { 1: true, 2: true } and updated adding/deleting tab IDs - not a required change but testing is easier (no indexOf needed).

em99 commented 1 year ago

Hi @AWolf81, nice to see some progress! hope to be able to update this extension soon!

AWolf81 commented 1 year ago

Hi @em99, I think I need some more days before the release.

Browser back button is also causing the issue with multiple windows. I need to check how this was fixed before. I also like to fix this.

Overall the behaviour is a lot better in the next version.

AWolf81 commented 1 year ago

:rocket: The new version is available in Mozilla extension gallery. Version 0.101.0 should fix the issue with the multiple windows.

Please test the version and let me know if this issue is fixed.

One issue I've noticed during testing: Dragging & dropping a tab to a new tab, sometimes requires a page reload to add the links. I'll check & fix this next week.

Update 02.07.2023 - I couldn't trigger the behavior again (dev version and 0.101.0). If I see the issue again, I'll create a separate issue for it.