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

Few minor improvements for Waterfox Current #1213

Closed
wants to merge 29 commits into from

Conversation

hawkeye116477
Copy link
Contributor

@hawkeye116477 hawkeye116477 commented Oct 19, 2019

Langpacks are from http://ftp.mozilla.org/pub/firefox/releases/68.1.0esr/linux-x86_64/xpi/ and little modified by me (changed Firefox to Waterfox, etc => hawkeye116477/LanguagePacksForWaterfox@957efac). For langpacks we may need maybe some makefile to put them in correct dir, I tried to add them in browser/features, but if they are on that dir, seems not working, however they work in browser/extensions, but are disabled and can't be enabled until I switch extensions.autoDisableScopes to 3.

I also added Unity/Global Menu support made by Canonical/Ubuntu Mozilla Team (https://bazaar.launchpad.net/~mozillateam/firefox/firefox.xenial/view/1347/debian/patches/unity-menubar.patch) and path to the running binary to about:support.

@grahamperrin
Copy link

grahamperrin commented Oct 19, 2019

Is the intention here to bundle (include) language packs?

If #1055 (comment) bugging about:preferences can be fixed, then I reckon it'll be better/simpler to have language packs on demand.

(Recent releases of Firefox take a very smart, effective approach. Quite unlike things as they were a few years ago.)

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Oct 19, 2019

@grahamperrin Maybe, just like for Classic.
If #1055 (comment) bugging about:preferences can be fixed, then I reckon it'll be better/simpler to have language packs on demand.
Putting link to AMO might be not the best thing, cuz some strings needs to be changed for Waterfox.
Better probably will be to put them on some own addons store (Addons.Waterfox.Net)/Waterfox's site. For Classic similar thing couldn't be done with provided update url on manifest? As I remember that was some time ago and then it came bundled.

* Restart button in PanelUI and Menu
* Copy current tab URL
* Copy all tab URLs
@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Oct 23, 2019

Small update:
Similarly as for Classic (#1203 (comment)), but prefs are on other place. I also hidden default not translated „Restart (developer)" menu item and replaced it with new which also has optional restart confirmation dialog. But maybe restart menu icon needs some improvement.
Screenshot_20191023_171636

@grahamperrin

This comment has been minimized.

@laniakea64
Copy link

Hi @hawkeye116477 , do these new browser.restart.* prefs apply to all restarts of Waterfox, or only when using the restart menu items added by this PR? If the latter, would it be possible to rename the prefs (maybe to browser.restart_menu.*) and clarify the about:preferences labels to avoid confusion?

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Oct 24, 2019

@laniakea64 That prefs applies only to restart menu items, similarly as it was on Cyberfox.

would it be possible to rename the prefs (maybe to browser.restart_menu.*) and clarify the about:preferences labels to avoid confusion?
Ok, I'll change that later.

@Peacock365
Copy link

Peacock365 commented Oct 24, 2019

Hey hawkeye116477,

would it be possible to also reinstate the Pop-ups section Cyberfox had? I mean the pop-up, graphics, and javascript settings you can see here:

https://i.computer-bild.de/imgs/4/9/0/7/8/6/2/Screenshot-7-Cyberfox-642x576-5a6b663220ecccae.jpg

After all, the about:config settings dom.popup_allowed_events (for pop-up windows - EDIT: unnecessary to restore this one, still exists under about:preferences#privacy, that leaves just graphics and Javascript), javascript.enabled (for javascript), and permissions.default.image (for graphics) still exist, so it shouldn't be hard to bring those settings back.

You would be an absolute hero if something like that were doable for you.

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Oct 25, 2019

@laniakea64 Changed
Screenshot_20191025_185139

@Peacock365 I'l look into that later.

@MrAlex94
Copy link
Collaborator

Right, so Mozilla have been testing bundling different languages at build time the same way they do the default locale. I’m going to figure out how to do that because the language packs inflate the binaries size by huge amounts when pre compressed.

https://firefox-source-docs.mozilla.org/build/buildsystem/locales.html

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Oct 29, 2019

@MrAlex94 Isn't better to just put langpacks separately from application on some addon store, just like Pale Moon team did?

Anyway, you can accept this PR, all should be good 😄.

@Peacock365
Screenshot_20191029_231541
Screenshot_20191029_231603

@Peacock365
Copy link

@hawkeye116477 Fantastic work!

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Nov 3, 2019

Small preview what I done:
Screenshot_20191103_190456
Screenshot_20191104_153750

If I want to also move some settings to another categories, make more categories, mainly move from general to another categories, little like on Vivaldi, should I do that in current preferences or make „new order" toggleable just like on Classic?

Recommendations doesn't work currently, so disable it
@grahamperrin
Copy link

Remove annoying message about private browsing on addons page

From the commit, I can't visualise the message. Can you provide a screenshot?

Thanks.

@hawkeye116477
Copy link
Contributor Author

@grahamperrin Set extensions.htmlaboutaddons.enabled to false and you'll see that message.

@grahamperrin
Copy link

The purple indicators?

2019-11-11 12:19:13

2019-11-11 12:25:23

It's important to not lose those completely.

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Nov 11, 2019

@grahamperrin Not purple, I mean that message „Waterfox Current is changing...".

@grahamperrin
Copy link

If that's removed, then what's left to educate the user?

In Firefox 70.0.1, a click on a purple icon of privacy refers to e.g.:

https://support.mozilla.org/1/firefox/70.0.1/FreeBSD/en-GB/extensions-pb

– which redirects to:

https://support.mozilla.org/en-US/kb/extensions-private-browsing?as=u&utm_source=inproduct

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Nov 11, 2019

@grahamperrin @Peacock365 requested, so I removed, but users will manage 😄
There is also inactive link to add-ons supports.
Maybe I'l look to that later and restore that links, which currently refer to nothing.

@Peacock365
Copy link

Peacock365 commented Nov 11, 2019

Uhm, @grahamperrin, this silly notification takes up a whole lot of screen real estate. This is unacceptable in about:addons, which is meant to be a rather compact overview of your add-ons. Also, the new HTML about:addons page doesn't display this notification, either. It's also unacceptable to have that notification there for the entirety of the next year. When you click on an add-on, you can choose whether to enable it in private browsing windows or not, the switch is rather self-explanatory as well.

Copy link
Collaborator

@MrAlex94 MrAlex94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm weary about adding this change - which is why I didn't do it in the first place. brandShortName is used quite often throughout the codebase, and I don't know how this might break users browser space. It has happened before where branding changes have messed things up, so this will need extensive testing.

MrAlex94
MrAlex94 previously approved these changes Nov 11, 2019
@MrAlex94 MrAlex94 dismissed their stale review November 11, 2019 22:53

I didn't approve of all changes.

@hawkeye116477
Copy link
Contributor Author

hawkeye116477 commented Nov 12, 2019

@MrAlex94 So should I revert 46ac944? Anyway without that change - on Linux is icon mish mash (#1211), there is also not distinctive title on Profile Manager (#1247).

this will need extensive testing.
Isn't that the next generation of Waterfox for testing?

Is there something another except this change which you didn't approve?

@MrAlex94
Copy link
Collaborator

Is there something another except this change which you didn't approve?

Not yet, but I need to test each change thoroughly. It's taking a while because there are a lot of changes here.

@hawkeye116477
Copy link
Contributor Author

I'd say not that a lot, only a lot code. Anyway, ok.

@MrAlex94
Copy link
Collaborator

MrAlex94 commented Nov 12, 2019 via email

@grahamperrin
Copy link

… this silly notification takes up a whole lot of screen real estate …

Agreed re: the use of space, which is why I drew attention to the sensible Firefox behaviour in response to clicks on the small purple icons.

@MrAlex94
Copy link
Collaborator

Okay, have been testing and all seems well. Will pull in a bit.

document.getElementById("appMenu-restart-button").hidden = false;
}
} catch (e) {
throw new Error("We're sorry but something has gone wrong with 'browser.restart_menu.showpanelmenubtn'" + e);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error keeps getting thrown when accessing the hamburger menu on macOS. Tested with both restart button enabled/disabled.

JavaScript error: chrome://browser/content/customizableui/panelUI.js, line 286: Error: We're sorry but something has gone wrong with 'browser.restart_menu.showpanelmenubtn'TypeError: document.getElementById(...) is null

Copy link
Contributor Author

@hawkeye116477 hawkeye116477 Nov 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see the problem, I added appMenu-restart-button inside of condition #ifndef XP_MACOSX, so it's not available for Mac.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for the fix.

@MrAlex94
Copy link
Collaborator

MrAlex94 commented Nov 20, 2019

Since I used most of the commits here (but not all), I git cherry-pick'ed the commits and then pushed them to the current branch. It doesn't look like GitHub knows how to handle partial commit pulls.

@Peacock365
Copy link

@hawkeye116477 @MrAlex94

I've noticed that, when one of the australized themes (curved tabs) is used, the new tab button in tab bar is squared, I suppose this is not intended to be so, since the new tab button behind the tabs was rounded in Waterfox 68b1... Please fix this cosmetic issue.

@hawkeye116477
Copy link
Contributor Author

@MrAlex94 In case of 2ba3e02, if you wanted FF-like behaviour, then browser.statusbar.mode should be set to 1, cuz 0 disables showing any status.

@Peacock365
Copy link

Kind reminder that the "new tab button cosmetic issue" still is not fixed.

MrAlex94 added a commit that referenced this pull request Dec 10, 2019
[current] Fixes to commits related to PR #1213
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.

5 participants