Skip to content
Please note that GitHub no longer supports your web browser.

We recommend upgrading to the latest Google Chrome or Firefox.

Learn more
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idref translator not working in Firefox, ok in Scaffold #1610

Open
symac opened this issue Mar 28, 2018 · 5 comments

Comments

@symac
Copy link
Contributor

commented Mar 28, 2018

This has been discussed in the pull request that created the translator : #1545
The issue is that detectweb is working fine when tested from scaffold, but with the Firefox connector, the icon does not show up (well it appears sometimes but not always). @adam3smith confirmed on the PR comment, I am adding this issue so that we don't forget it and maybe someone will have more idea about it.

@symac

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2019

@adam3smith this issue is still pending and I don't have any clue on the best way to fix this. Do you have any idea if something currently being worked on in Zotero might be leading to a solution or if something could be done on idref.fr site ? I'm in contact with them so if you have any clue, I can forward to them.

Thanks,
Sylvain

@Francois-Mistral

This comment has been minimized.

Copy link

commented Jun 25, 2019

Hello @adam3smith,
would you have any idea to fix this issue : it's so close to work, that it's a shame !
Your help would be really appreciate.
Best regards
François

@adam3smith this issue is still pending and I don't have any clue on the best way to fix this. Do you have any idea if something currently being worked on in Zotero might be leading to a solution or if something could be done on idref.fr site ? I'm in contact with them so if you have any clue, I can forward to them.

Thanks,
Sylvain

@symac

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2019

Coming back to this issue it appears that it is because of the getSearchResults that is called from detectWeb. In fact when using scaffold, this function is fired when the developer is clicking on the eye icon, and at this time, usually, the asynchronous loading of the result lists is finished so it is working fine.
But when Firefox runs detectWeb, the result list on an idref record is still loading and getSearchResults returns false.

This detect issue seems to be fixed by replacing detectWeb by replacing:

function detectWeb(doc, url) { 
	if (ZU.xpathText(doc, '//div[@class="detail_bloc_biblio"]') && getSearchResults(doc, true)) {
		return "multiple";	
	}
}

by:

function detectWeb(doc, url) { 
	if (ZU.xpathText(doc, '//div[@class="detail_bloc_biblio"]')) {
		return "multiple";	
	}
}

The only remaining issue is if the user clicks on the folder icon before the result list is fully loaded. I am not sure it will often happen in real life but I'd like to be able to find a proper solution to fix it. The best one I have found so far is to add at the beginning of the doWeb function the following code:

	searchResultsReady = getSearchResults(doc, true);
	if (!searchResultsReady) {
		var timeout = Date.now() + 5000;
		while (Date.now() < timeout){}
	}

It will add a delay of 5 seconds in case the user is clicking on the icon too early before the result list is loaded. It's far from perfect but it should fix most problematic cases.

I will leave that there for a while in case anybody has better advice to fix this, then I will propose a PR based on these ideas.

@adam3smith

This comment has been minimized.

Copy link
Collaborator

commented Oct 21, 2019

Haven't looked at this for a long time, but is there a reason ZU.monitorDOMChanges wouldn't be an option? That's much better than a timed delay, which can be annoying and unreliable.

@symac

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2019

@adam3smith I have read somewhere that ZU.monitorDOMChanges was available only for detectWeb and our current issue is with doWeb. DetectWeb can return multiple without waiting for the full list to be loaded, so we need a way to wait within doWeb in case user clicks quickly, but I have not found (yet?) a better solution than the while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.