-
Notifications
You must be signed in to change notification settings - Fork 770
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
DOI web translator: Offer the current page as a choice among the multiple #3130
base: master
Are you sure you want to change the base?
DOI web translator: Offer the current page as a choice among the multiple #3130
Conversation
|
I feel that a potentially easier de-dup approach is to compare the distances in the |
Hmm, no, I think the title may actually work better than URL, because the URL of the special "current page" item may look quite unlike the URL reported by the DOI-generated item. Example: https://physics.aps.org/articles/v16/127 EM-generated page item has the same URL, but its own DOI (mentioned on the page in a href hence picked up by the DOI web translator) resolves to |
@AbeJellinek, following your suggestions I added a de-dup routine that checks whether the current web page's URL or title matches a DOI-resolved item's, based on a normalized Levenshtein distance. The metric and threshold are rather arbitrary but I think they'll catch the obvious dups. |
I've been meaning to change that for about a decade, so "Webpage" is fine, but it should really be a localized string from the connector. Can we just return the current URL and leave it to the connector to sub in a different string there if it matches? |
A personal wish of mine is to have easy access to proper localization in translator scripts. It would be great if the Connector could support that.
This is certainly doable. Until the connector can do the substitution, it will fall back to displaying the URL, which may look a bit weird but could be a bigger problem with screen readers. |
It should just be the current page title, no? |
Right now, it is the text Example: |
Yeah, so it should just be the page title, and the connector can change it if the URL matches the current page. |
What else does the connector need, in order to do this substitution? Does it need the translator to pass a { string: |
Unless I'm misunderstanding something, it doesn't need to do anything other than put the current page first. |
select[doi] = item.title || "[" + item.DOI + "]"; | ||
} | ||
} | ||
let select = buildSelections(items); | ||
Zotero.selectItems(select, function (selectedDOIs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To continue the discussion from #3130 (comment) ...
I'm afraid I may be misunderstanding something.
My understanding is that we're discussing about the text that is to appear in the "Zotero Item Selector" popup for the special "current web page" item. The object select
here, passed to Zotero.selectItems()
, is responsible for what the user sees in the Item Selector. This object is a simple mapping of string (DOI) -> string (title as shown to the user in the Item Selector).
If I understand it, you would be saying that here what the translator should do was to put the current page's title as the value for the special item.
What I was asking was whether this could be sufficient for the "text substitution" (into the localized string for "current web page") to be implemented in the Connector. I was asking because I didn't understand how that could be done without any URL info present in the select
object. But I may be missing something.
Maybe an empty string as the key instead? |
Here's more context about why I was asking It's clear that the first argument (mapping of selectable items) passed to But for @dstillman's idea to work smoothly with translators, I think a { string: Item } mapping may be necessary here, if text substitution for the title is to be done in the Connector. @AbeJellinek, what do you mean by "as key"? |
Based on how pre-selection is implemented (see e.g. PubMed.js) Lines 172 to 175 in 5187948
I created a PoC for a mechanism to tell the item selector about the current webpage among them. The following is a demo in Scaffold: https://github.com/zotero/zotero/compare/40b30f2b632d54bc31f94715d4251687af38744c...zoe-translates:zotero:PoC-make-it-possible-to-identify-current-page-by-translator-for-multiple-selection?expand=1 This puts the onus on the translator, so on the connector side there's minimal change to existing code and little new logic. Below is how it looks in Item Selector Current page is already one of the items referred to by DOI on the page itself: Current page as an option in addition to the DOI-resolved items (always the first one): On the translator side, it is supposed to be like this: Lines 281 to 286 in bb8479e
Of course, the ultimate goal is to do i18n properly, without using a hard-coded text label. @dstillman @AbeJellinek I wonder how you feel about this solution? |
So the issues here as I see them are:
I'm fine with the approach @zoe-translates proposes above, but I sort of like the idea that an But why not use the page URL as the key, just for this special case? The translator would call Zotero.selectItems({
'https://www.example.com/some/page': 'Page title',
'10.48905/some/other/DOI': 'Title of a paper that got mentioned',
/* and so on */
}); And the |
Thanks for the explanation @AbeJellinek Let me try to summarize the current proposals
Notice that to take advantage of either, most translator code would have to be changed anyway, because you have to put the current page in the selectable items as an offer and be ready to handle it in the translator. Some pros and cons of these proposals that I can think of -
It seems to me that the first approach is probably better because its benefits are more concrete. BTW, you mentioned " |
Yeah, fair. The page URL changing is a concern, but I think in practice the likelihood that it changes between when |
Alright! I think we have a plan. Thanks for these discussions! @AbeJellinek So I'll go on to make the necessary changes to this DOI translator (which will still be backward-compatible even before Connector gets updated). Meanwhile, do you mind if I go on to update Connector, Scaffold, and translation-server with the proposed changes? |
In anticipation of updates to the Connector in the item-selector, where a special key -- the translation-initiating page's URL -- will be used to label it as the current page with proper localization, changes are made here to take advantage of that upcoming feature, and display non-localized title label as fallback before the Connector is updated. (Cf. zotero#3130 (comment)) The logic of doing so is a bit non-trivial, and the code is now more extensively commented. Some minor simplifications are done in passing.
Go for it. |
I serialized the DOI Content Negotiation requests. Can you rebase on the latest DOI.js? |
…iple If the DOI translator turns out to be the one that matches the current page, and if the DOI (this) translator detects the item type as "multiple", offer the current page as one of the choices (indeed, the first choice). In the item selection dialog, this special choice will show up as "Current Webpage", followed by the page title in parenthesis if the title is present. The specific situation that triggers this behavior is the following 1. None other than the DOI web translator (lowest priority) matches the current page 2. The current page itself is not the main resource identified by DOI present on the current page (e.g. a page with its own DOI in URL) Resolves zotero#3126.
…ultiple Use a dissimilarity metric based on item URL and title to de-duplicate the special "current web page" item. A test case is added for this scenario. Also, add a visual cue (text "possibly current web page" in parentheses after the title) in the item selection dialog for the choice that may be referring to the current web page.
…esult Also, prefer "web page" to "webpage" in human-readable text
In anticipation of updates to the Connector in the item-selector, where a special key -- the translation-initiating page's URL -- will be used to label it as the current page with proper localization, changes are made here to take advantage of that upcoming feature, and display non-localized title label as fallback before the Connector is updated. (Cf. zotero#3130 (comment)) The logic of doing so is a bit non-trivial, and the code is now more extensively commented. Some minor simplifications are done in passing.
…tions - Guard against the very rare case when empty object could be passed to Z.selectItems() - Eliminate superfluous intermediate variable
- Check if URL domains form subdomains rather than simple substrings - Path comparison is verbatim up to any final trailing slash - Title comparison is done after stripping out punctuations
deff79d
to
676e9a8
Compare
If the DOI translator turns out to be the one that matches the current page, and if the DOI (this) translator detects the item type as "multiple", offer the current page as one of the choices (indeed, the first choice).
In the item selection dialog, this special choice will show up as "Current Webpage", followed by the page title in parenthesis if the title is present.
The specific situation that triggers this behavior is the following
Resolves #3126.