Skip to content
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

OlgaTPark/tenfourfox#14 — Add a Keyboard Shortcut to Reader Mode #629

Merged

Conversation

OlgaTPark
Copy link
Contributor

@OlgaTPark OlgaTPark commented Oct 7, 2020

Here are the changes discussed in OlgaTPark#14 needed to add a keyboard shortcut to Reader Mode (the patches are based on PowerPC TenFourFox FPR6 because I tested them in OlgaFox FPR6).
This commit doesn't include M1438308 which changes the shortcut on Windows to F9.
I also didn't included M1480415 which concerns a Windows screen reader that cannot "see" the reader button in the toolbar (but I doesn't expect a better situation on Mac OS X).

If you want, I can also add the changes for the pdfjs.display.use_document_fonts=true setting to this pull request.

Concerning localizations, two files are affected:

OlgaTPark@8c65cb0#diff-38fb3065c40acfe9cd97e7cd67908052R102-R113
/browser/locales/en-US/chrome/browser/browser.dtd

 <!ENTITY enterFullScreenCmd.accesskey "F">
 <!ENTITY exitFullScreenCmd.label "Exit Full Screen">
 <!ENTITY exitFullScreenCmd.accesskey "F">
 <!ENTITY fullScreenCmd.label "Full Screen">
 <!ENTITY fullScreenCmd.accesskey "F">
 <!ENTITY fullScreenCmd.macCommandKey "f">
 <!ENTITY showAllTabsCmd.label "Show All Tabs">
 <!ENTITY showAllTabsCmd.accesskey "A">
+<!ENTITY toggleReaderMode.key "R">
 
 <!ENTITY fxaSignIn.label "Sign in to &syncBrand.shortName.label;">
 <!ENTITY fxaSignedIn.tooltip "Open &syncBrand.shortName.label; preferences">
 <!ENTITY fxaSignInError.label "Reconnect to &syncBrand.shortName.label;">
 <!ENTITY fxaUnverified.label "Verify Your Account">

(which ends up in {TenFourFox.app/Content/Resources}/browser/chrome/en-US/locale/browser/browser.dtd).
Here, the change is the same for every locale (checked in Mozilla XPIs).

And:
OlgaTPark@8c65cb0#diff-0b08792398c408a93dd2a72b4b61cab5L286-L295
/browser/locales/en-US/chrome/browser/browser.properties

 # Unified Back-/Forward Popup
 tabHistory.current=Stay on this page
 tabHistory.goBack=Go back to this page
 tabHistory.goForward=Go forward to this page
 
 # URL Bar
 pasteAndGo.label=Paste & Go
+# LOCALIZATION NOTE (reader-mode-button.tooltip):
+# %S is the keyboard shortcut for entering/exiting reader view
+reader-mode-button.tooltip=Toggle reader view (%S)
  
 # Block autorefresh
 refreshBlocked.goButton=Allow

(which ends up in {TenFourFox.app/Content/Resources}/browser/chrome/en-US/locale/browser/browser.properties).

I extracted the translated strings for every locale supported by TenFourFox from http://ftp.mozilla.org/pub/firefox/releases/60.0/mac/xpi/:

Locale Translated string
de reader-mode-button.tooltip=Leseansicht umschalten (%S)
en-US reader-mode-button.tooltip=Toggle reader view (%S)
es-ES reader-mode-button.tooltip=Cambiar vista de lectura (%S)
fi reader-mode-button.tooltip=Näytä/piilota lukunäkymä (%S)
fr reader-mode-button.tooltip=Activer/Désactiver le mode lecture (%S)
it reader-mode-button.tooltip = Attiva/disattiva Modalità lettura (%S)
ko reader-mode-button.tooltip=읽기 모드 토글(%S)
pl reader-mode-button.tooltip=Przełącz poprawianie czytelności (%S)
ru reader-mode-button.tooltip=Включить/отключить Вид для чтения (%S)
sv-SE reader-mode-button.tooltip=Växla läsvy (%S)
tr reader-mode-button.tooltip=Okuyucu görünümünü aç/kapat (%S)
zh-CN reader-mode-button.tooltip=切换阅读器视图 (%S)

And for my forgiveness, I can already translate the strings from #328 (comment) in French:

  • "Domain" => "Domaine"
    • I greped it in the localizations to see if it's already somewhere else and I've found:
      • TenFourFox.app/Contents/Resources/chrome/fr/locale/fr/cookie/cookieAcceptDialog.properties: domainColon=Domaine :
      • TenFourFox.app/Contents/Resources/browser/chrome/fr/locale/fr/devtools/client/netmonitor.dtd: <!ENTITY netmonitorUI.toolbar.domain "Domaine">
      • TenFourFox.app/Contents/Resources/browser/chrome/fr/locale/browser/preferences/cookies.dtd: <!ENTITY props.domain.label "Hôte :"> (This variant has a « Hostname » meaning)
  • "Configure Site Preferences" => "Configurer les préférences du site"
  • "Always Open in Reader View" => "Toujours ouvrir en mode lecture"

…er Mode

This commit includes the following Mozilla changes:   
 - Bug 1144749 - Add keyboard shortcut to open ReaderView.
 - Bug 1344211 - Reader view shortcut should be shown in 'View' menu.
 - Bug 1441788 - Expose Reader View shortcut in the address bar button's tooltip.

Please note that Windows users will also need https://bugzilla.mozilla.org/show_bug.cgi?id=1438308 (affects localizations) which isn't included here.

References:
https://bugzilla.mozilla.org/show_bug.cgi?id=1144749
https://hg.mozilla.org/mozilla-central/rev/c2849a432eee
https://bugzilla.mozilla.org/show_bug.cgi?id=1344211
https://hg.mozilla.org/mozilla-central/rev/9c177d80ee84
https://bugzilla.mozilla.org/show_bug.cgi?id=1441788
https://hg.mozilla.org/mozilla-central/rev/5ed5cde4cb2c
@classilla
Copy link
Owner

Thanks for doing this. I was expecting a menu item, though (or at least to unhide the existing one). I guess I don't mind this being attached to the bar icon too, but it seems like it should be in both places and the string changed to "Enter Reader View" (capitalized).

Adding @chris-chtrusch about the locale change.

@chris-chtrusch
Copy link
Collaborator

For which FPR version is this planned?

@classilla
Copy link
Owner

Well, I guess that's a good question. I'd hate to ship locale updates back to back and even though this is a really low risk change I'd prefer not to ship a new feature without a beta. The two options are to hold the Enable JavaScript feature over for the next release, or to hold this one for another locale change. @NapalmSauce , do you have any strong feelings here?

@NapalmSauce
Copy link
Contributor

Well, I have my builds, so I don't mind if you hold the JS toggle for the next release, really. Thanks again!

@chris-chtrusch
Copy link
Collaborator

chris-chtrusch commented Oct 7, 2020

I have no problem making back-to-back langpack installers. Do you think people will be confused if they have to download yet another installer that only lasts one version?

The only thing I don't really want to do is to make the langpacks 'blindly', i.e. without an English (beta) version of the browser that actually has the new items working.

@classilla
Copy link
Owner

@chris-chtrusch I agree with that too (that's why we have betas). @NapalmSauce , thank you for being flexible! Rather than do a full backout I'll just neuter the Enable JavaScript option and add it back for FPR29b1. I will merge this PR then as well with the requested change(s).

@classilla classilla merged commit 61f16fc into classilla:master Oct 23, 2020
classilla added a commit that referenced this pull request Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants