Add host permission prompts for Safari - #631
Conversation
e8d32d6 to
6873733
Compare
- Upon first enabling the Zotero Connector to run on any page, prompt about required permissions for localhost, and suggested permissions for all websites - Once per browser start if localhost permissions are missing prompt about them, and trigger a ping to Zotero, which will show a Safari permission dialog (only if Connector is allowed to run on a given page) - If attempting to save without required permissions, prompt - If localhost permissions are enabled, but Zotero is offline, prompt about repo.zotero.org permissions required for translators - If saving with Zotero offline, prompt about api.zotero.org permissions required for saving to web library - If https://*/* permissions are missing, prompt about it every time the user opens the preference pane
Opening the preferences pane without localhost access triggered two concurrent 'confirm' prompts -- the ClientStatus online check's localhost explanation and the pane's all-hosts recommendation. The status request could also trigger Safari's native localhost permission dialog before the pane's explanatory prompt was displayed. - Serialize Zotero.HostPermissions.prompt() through a per-tab queue so concurrent callers show modals sequentially, without a modal left open in one tab blocking prompts in other tabs, and with permissions re-checked when each queued prompt runs - Forward permissionPromptShown through checkIsOnline() so the pane's status check can skip its own localhost prompt while still making the request that can trigger Safari's native permission dialog - Fold the localhost explanation into the pane's combined prompt alongside the all-hosts recommendation - Run the initial client status check only after the pane's permission prompt has been dismissed
When a permissions prompt covered both required domains and the
all-hosts recommendation, it showed two nearly identical 'To enable
this, open Safari Settings...' paragraphs. Explanations of the missing
access now come first, followed by one instructions paragraph covering
everything being requested.
Domain lists in the instructions are formatted with Intl.ListFormat
('repo.zotero.org and api.zotero.org'), which uses the locale-appropriate
conjunction, instead of as a bare comma-separated list mid-sentence.
Saving with 127.0.0.1 access denied showed an OK-only prompt with Safari Settings instructions and then silently aborted, with no way to save to the online library instead. And when the permission state was Ask, the Settings instructions were moot, since the request that followed triggered Safari's own permission dialog. The save flow is now: - Before the status request, a prompt explains that localhost access is required and points to Safari's upcoming permission dialog instead of Safari Settings (also on the once-per-session page-load prompt, which pings right after) - When that prompt also recommends all-websites access, the recommendation likewise points at the dialog's 'Remember for other websites' checkbox rather than Safari Settings, and mentions the save button updating for each webpage and more reliable saving, which matter to users more than site-specific citation export - If the request fails but access was granted in Safari's dialog, ping again silently, since the blocked request says nothing about Zotero's status - If the request fails with access still missing, show the offline dialog with the explanation adjusted to denied localhost access -- Safari Settings instructions plus the online-library alternative -- with the usual Try Again/Cancel/Enable Saving to Online Library buttons. Only users who have actually denied access reach this dialog. - Choosing the online library sets the firstSaveToServer pref as in the Zotero-offline case. The pref only governs the fallback prompt when Zotero is unreachable, so direct saving still resumes automatically once localhost access is granted and Zotero is running. - Retries skip the pre-request explanation, which has already been shown
Safari runs content scripts only on sites where the user has granted access. On other sites, it always showed the gray webpage icon, and clicking the button enabled the Connector on the site rather than saving. Skip per-tab icon updates until a content script reports detection results, and swap the manifest's default icon to the Z in the Safari build. The Z is rendered from z.svg in the Zotero 7 client (chrome/skin/default/zotero/z.svg), which isn't yet in the Zotero (6) submodule, at 15px and 30px centered on 16px and 32px canvases so it doesn't fill the full icon space. Nearly transparent corner pixels keep Safari from trimming the padding and scaling the glyph back up.
checkIsOnline() now returns null for passive checks too when Safari localhost access is missing, and the icon for excluded pages (zotero.org, extension pages) shows the plain Z with a 'Zotero Connector' tooltip in that case instead of claiming that Zotero is offline.
4001127 to
3adfa8c
Compare
On document load, the Google Docs integration requests repo.zotero.org/settings to decide whether to use the Google Docs API client, and the request triggers Safari's own permission prompt, which can appear over the Connector's site-access explanation prompt. The integration uses this method to skip the request when repo.zotero.org access is missing on Safari.
- Skip the settings request without repo.zotero.org permission on Safari - Don't add the Zotero menu and toolbar button if they already exist
The explanation prompt appears for any blocked interaction with Zotero, including Google Docs citation commands, so don't describe 127.0.0.1 access as specific to saving, and explain what the address is.
…uests When a Google Docs integration command failed because Safari denied access to 127.0.0.1, the error dialog asked whether Zotero was running, which is unknown and misleading in that state. Show the localhost explanation with Safari Settings instructions instead, matching the save flow.
…locked Safari doesn't show its permission dialog for a request when the access is set to Deny, so after a request has been blocked with localhost access still missing, the explanation saying Safari may ask was wrong, and each retry showed it again followed by the failure dialog with Safari Settings instructions. Remember that state and skip straight to the failure dialog for further requests.
Safari's permission dialog is the one place all-websites access can be granted with a single click, so recommend "Remember for other websites" when explaining a request that can trigger the dialog. The recommendation is dropped automatically once all-websites access is granted.
…e ever The page-load recommendation was gated by a stored pref, and extension storage survives reinstallation on Safari, so a user who dismissed the prompt once would never see the recommendation on page load again. The page-load prompt flow already runs once per browser session, so drop the pref and rely on that.
…on grant Safari injects the extension's content scripts into already-loaded matching pages when the user grants new host permissions, e.g., by checking "Remember for other websites" in its permission dialog. The scripts run in the same content world as the running copy, recreating the Zotero namespace and throwing on top-level let and const redeclarations, which leaves the page with a broken mix of the two copies. (On Google Docs, this resulted in a broken Zotero menu after granting permissions for 127.0.0.1 after granting access to Google Docs itself.) Detect the second injection via a marker on the document and reload the page for a clean single injection, which is also how Safari itself enables the extension on a page via the toolbar button. Safari can also inject the scripts twice while a page is loading, which a reload cannot fix, so only reload when the user has just granted a host permission, and at most once a minute in a tab. Safari doesn't notify the extension of grants made in its permission dialog or Safari Settings, so the background tracks the permission state and records a grant when a permission appears that the last seen state lacked, refreshing on an interval so that revocations are recorded too and a revoked-and-regranted permission is still detected. A detected localhost grant also reenables requests blocked while access was missing.
Safari doesn't run content scripts in tabs that are already open when the user grants site access, so the save button and context menu items sent messages to pages with no scripts and silently did nothing. Inject the scripts on demand, as detected via a ping, and give translator detection a moment to report before a save mode is chosen.
The instructions said "To enable this" even when prompting for multiple domains, e.g., repo.zotero.org and api.zotero.org when saving with Zotero offline. Split the string into singular and plural variants chosen by the number of missing domains.
injectTranslationScripts() returned before its cleanup when a ping showed the scripts were already present, permanently caching the per-tab in-progress marker, so all later calls for the tab returned the cached promise without pinging or injecting. Chromium and Firefox inject content scripts on every page, so the skipped injection didn't matter there, but on Safari a tab can reach a page without scripts, e.g., by navigating to an ungranted site that the user then grants access to, and the on-demand injection for user actions would silently do nothing for the rest of the tab's life. Run the cleanup on every path.
3adfa8c to
4db8be5
Compare
|
OK, made a bunch of tweaks to the dialogs, and did a huge amount of testing to try to get the prompt experience right in Safari with different combinations of initial permissions. I'm going to merge to get this into a beta before Z10, but let me know if anything looks wrong. |
|
Merged. Re: the gdocs submodule commit for repo.zotero.org/settings, we could probably just remove that request instead now that we're always using v2. |
| (function() { | ||
| // Safari injects the extension's content scripts into already-loaded matching pages when the | ||
| // user grants new host permissions. The scripts run in the same content world as the first | ||
| // copy, which recreates the Zotero namespace and throws on top-level let and const |
There was a problem hiding this comment.
I don't understand this. If the host has no permission to run on the page, there is no "first copy", because nothing gets injected until permission is granted. Is this for multiple tabs with the same host? Or did you observe this for a single tab?
In my testing I didn't see any duplicate injection behavior, but this explanation doesn't make sense to me.
There was a problem hiding this comment.
This was trying to get Google Docs to work on first run. If no host permissions, you have a Z (previously gray webpage icon) on the save button. If you click that and grant access only to docs.google.com but not 127.0.0.1, we then show an alert about 127.0.0.1. After you click "OK", we trigger the Safari access prompt for 127.0.0.1. After granting access, the new Zotero menu in Google Docs wouldn't work, and this (including a couple extra debugging lines) showed up in the console:
(Initially two Zotero menus would show, because it was injecting each time, but we now prevent that by checking for the DOM element.)
There was a problem hiding this comment.
I mean google docs menu injection depends on Zotero scripts being loaded before their editor finishes initializing, so for google docs permission granting it would make sense to reload the page as an exception. But I've not seen duplicate script injection or that causing issues for translation in other places.
Either way, this comment seems incorrect, overly-verbose, and likely to further trip up LLMs and people in what goes on here, or for what reasons.
There was a problem hiding this comment.
Easy to see the duplicate injection on any site:
- Set all access permissions for the temp extension to Ask.
- Uninstall the extension.
- Restart Safari.
- Load forums.zotero.org, show the page console, and enable Persist Logs.
- Add the temporary extension and reload the forums tab.
- Click the Z and grant access to forums.zotero.org for one day.
- Get the 127.0.0.1 warning. Click OK.
- In the Safari access prompt for 127.0.0.1, enable "Remember for other websites" and click one of the allow options. It doesn't happen if you don't enable "Remember for other websites".
I didn't test all the ways it might break things on non–Google Docs pages, but it almost certainly would break something, and it doesn't seem like a state we want to operate in, so reloading is the safest option.
I've clarified the comment in c5a97be to note that it depends on "Remember for other websites".
| let tabInfo = Zotero.Connector_Browser.getTabInfo(tab.id); | ||
| for (let i = 0; i < 30 && !tabInfo.translators; i++) { | ||
| await Zotero.Promise.delay(100); | ||
| } |
There was a problem hiding this comment.
If a page has no translators, doesn't this cause a 3 second delay every save?
There was a problem hiding this comment.
No, tabInfo.translators with no detected translators is an empty array, so it's not falsy.
But if the user navigates during the loop, this wouldn't get the new value, which might result in a stall, so I'll change this to get the tab info again on each loop.
Sometimes the individual (or ALL) host permissions may be configured to deny, but will proceed anyway when requested via background page. This is even if the browser's own API will report the permission for a given host as missing. Sometimes it persists with Safari restarts, other times it clears with merely reloading the extension. As such, sometimes we are showing prompts for permissions for e.g. api.zotero.org, and then those requests succeed anyway.
We include the instructions on how to enable permissions for a given host in every prompt, since we don't know if a host we don't have permissions for is set to "Ask" or "Deny". If it's "Ask", Safari will follow our prompt by displaying its own native one, where the user can allow Zotero Connector the host permission. If it's "Deny", no such prompt is shown.