classilla / tenfourfox

Mozilla for Power Macintosh.
http://www.tenfourfox.com/
Other
270 stars 38 forks source link

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

Closed OlgaTPark closed 3 years ago

OlgaTPark commented 3 years ago

Here are the changes discussed in OlgaTPark/tenfourfox#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:

https://github.com/OlgaTPark/tenfourfox/commit/8c65cb0c773492a235d8b253301155e23b51130d#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:
https://github.com/OlgaTPark/tenfourfox/commit/8c65cb0c773492a235d8b253301155e23b51130d#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 https://github.com/classilla/tenfourfox/issues/328#issuecomment-703730518 in French:

classilla commented 3 years ago

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 commented 3 years ago

For which FPR version is this planned?

classilla commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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).