For #11404 : "Open all..." option

[Taknok]

Add the "Open all..." option as described in #11404 (similar to "Open all" present on firefox desktop).

This option open recursively all link in a bookmark folder, if there is no link in the folder, display a toast message with the error. All strings a retrieved from "l10n-mozilla firefox-desktop". Once links are opened, the tabs tray is shown.

Folder structure for this exemple:

How the option is shown in overflow menu:

When opening all:

When a folder is empty:

Pull Request checklist

• Tests: This PR includes thorough tests or an explanation of why it does not
• Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
• Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

1. click on Show All Checks,
2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
3. click on the "Fenix - assemble" task, then click "Run Artifacts".
4. the APK links should be on the left side of the screen, named for each CPU architecture

Close #11404
Fixes #11404
Fixes #11404
Fixes #11404

[firefoxci-taskcluster]

No Taskcluster jobs started for this pull request

The `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request.

[Taknok]

Can a reviewer tell me if this is a correct way to introduce new strings ? As both chats fenix an l10n in chat.mozilla.org couldn't tell me if I should push it 1st on l10n or fenix project.

[Mugurell]

Can a reviewer tell me if this is a correct way to introduce new strings ? As both chats fenix an l10n in chat.mozilla.org couldn't tell me if I should push it 1st on l10n or fenix project.

All new string resources are to be added only in Fenix values/strings.xml - only to the default strings file.
Localization is done automatically by a bot that will sync resources with Pontoon, we should not manually update localized strings in Fenix.

[Taknok]

Ok, so I should only set in values/strings.xml wait this string is imported in l10n, copy values on Pontoon from desktop to fenix and wait auto import here ?

[Mugurell]

Ok, so I should only set in values/strings.xml wait this string is imported in l10n, copy values on Pontoon from desktop to fenix and wait auto import here ?

Once the PR is merged with the changes in values/strings.xml the work on Fenix is done.
If you want to help with translations on Pontoon you can certainly do that and would be highly appreciated but that work is outside of the scope of the Fenix ticket.

[gabrielluong]

We probably want to check with Product/UX before proceeding.

[Taknok]

I thought that the interface modification was not that important, but ok. Should I contact them, fill an issue/a form or they will pass on their own ?

[amedyne]

@topotropic or @vesta0 Any thoughts on this?
@Taknok FYI. tagging them here for input.

[Taknok]

According to this documentation an engineer should contact the designer (desk or slack).

[Taknok]

Force-push to rebase PR onto main branch.

Any update about this UX review ?

[amedyne]

Talked to product managers about this PR, the request is to implement telemetry as part of this change. @gabrielluong or @Mugurell here's the Jira ticket: https://mozilla-hub.atlassian.net/browse/FNXV2-17873 for the change. Do you think you can take on this or mentor/guide @Taknok to implement?

[Taknok]

Thank you for your quick response. I cannot access the jira ticket, thus I cannot see what is asked. What should be implemented in the telemetry ? Create a special entry ? Count all tabs opened ? Just register one count even if multiples are opened ?

[amedyne]

Thank you for your quick response. I cannot access the jira ticket, thus I cannot see what is asked. What should be implemented in the telemetry ? Create a special entry ? Count all tabs opened ? Just register one count even if multiples are opened ?

The request is to register one count if a user selects the "Open All Bookmarks" action.

[Taknok]

Can I link "Open all" from the overflow menu (this PR) to the event Event.OpenedBookmarksInNewTabs which is currently used when a user selects multiple bookmark items and clicks open (which currently does not work with folders, this is why this issue and PR were created).

To sum up:
Event Event.OpenedBookmarksInNewTabs would be triggered when a user opens a folder tree dots menu and click "Open all" and/or when user select multiple items (no folders), click tree dots menu from selection bar and click "open in a new bookmark".

[eliserichards]

I think we should make a new Event for open all, just so we can differentiate. What do you think @Taknok?

[eliserichards]

e.g. Event.OpenAllBookmarksInNewTabs

[Taknok]

Here is the metric with Glean.

[Taknok]

Force-push to rebase PR onto main branch.

Any update about this ?

[Taknok]

Force-push to rebase PR onto main branch.
Update tests due to deletion of closeTabViaXButton.

Any update about this ?

[mergify]

This pull request has conflicts when rebasing. Could you fix it @Taknok? 🙏

[Taknok]

Fixed & force-push to rebase PR onto main branch.

[mergify]

This pull request has conflicts when rebasing. Could you fix it @Taknok? 🙏

[Mugurell]

Awesome work on this with cool functionality, telemetry, unit and ui tests 👍!

I think there is only one blocking issue: the new option to open all bookmarks should not appear if there are no bookmarks in the folder.

There are also some open questions which might be addressed in followup tickets:

• Whether we should have two new folder options: "Open all in new tabs" and "Open all in private tabs" and a confirmation on the strings.
• Whether we should show a warning dialog before opening a large number of tabs.

[rocketsroger]

This landed without any data-review. The probes are also set to never expire. What happened here?

[rocketsroger]

Another issue is a string was updated directly. A new string should have been added. Since there are multiple issues, I'll have to revert this. Let's land this with data-review and a expiry version.

[rocketsroger]

@Taknok unfortunately there are multiple issues here causing us to revert. Please reopen a new pr with two updates:

1. Create a data review for the telemetry (example) and tag the pr "need: data-review". Make sure to not use "never" as expiry.
2. Please create a new string if a string update is unavoidable. (example)

Please let me know if I can help. Thanks

[Taknok]

Sorry but I will be rude here.

First:

This landed without any data-review. The probes are also set to never expire. What happened here?
@rocketsroger

I set an expiration date for probes in the initial PR, but I was told to change it to never here :

Maybe they should be set to never expire
@Mugurell

Second:

Another issue is a string was updated directly. A new string should have been added. Since there are multiple issues, I'll have to revert this. Let's land this with data-review and a expiry version.
@rocketsroger

What string ? I only see line insert in this PR files app/src/main/res/values/strings.xml Do at least link the line with an issue.

Data-review ? Was not @eliserichards or @topotropic doing this ? I am so lost in this maze that this PR has been though...

Third:
This is totally not acceptable the way you all handle this. I am a benevolent contributor, but I feel handle like a infinite free disposable work force. I sorry but I am pissed off to the point I do not care if this is because Mozilla does not put enough energy/money/[what ever you want] in their mobile team. But I'm rebasing and resolving conflict on the same shitty basic feature for over a year !
I asked what should I do, I did everything that I was told. Finally it is accepted, and no, now I have to start over this hell ??? To do stuff initially done ??? So I am tired... so do it yourself, I am giving up.
This all thing convinced me to switch to Brave (I have been using Firefox since version 2 (2006 !)). Yes Firefox was one last alternative to big companies but Mozilla lost itself somewhere and I am profoundly sad about it.
Plus, now, on every approach on LinkedIn from Mozilla, I will simply reply a link of this PR !

[rocketsroger]

... But I'm rebasing and resolving conflict on the same shitty basic feature for over a year ! I asked what should I do, I did everything that I was told...

I understand your frustration. I would never expected you to know what's needed to land an issue and the reviewers should have given you proper instructions. However, my hands are tied in this case. At Mozilla we take privacy very seriously. Any data collected have to go through data-review. Any data that is collected without expiry needs product approval as well. I want to help you re-land this. Please put up a new PR when you're ready and I'll work with you (in a timely manner) to land this as soon as possible. Thank you,

[rocketsroger]

@Taknok another solution is I can make the modifications and land this PR with you as a co-author. I'll go through the data review for you as well. If you're OK with my name associated with the PR that is. Please let me know.

Thank you for your passion and your contribution.

[Taknok]

You can do the modifications you want, I am just not doing it anymore, I will even accept PR from other contributors to my branch if needed.

[rocketsroger]

You can do the modifications you want, I am just not doing it anymore, I will even accept PR from other contributors to my branch if needed.

I'll create a new PR with you as co-author. Thanks