[Toolbar Redesign] CustomTab menu items are not created when IncompleteToolbarRedesign feature flag is on
Categories
(Fenix :: Toolbar, defect, P2)
Tracking
(Not tracked)
People
(Reporter: harrisono, Assigned: harrisono)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fxdroid][toolbar:130])
Attachments
(6 files)
Normally, a custom tab in Slack will show the menu items for "Share via", "Copy link", and "Share in Slack".
When the Toolbar Redesign feature flag is enabled and the menu is opened in a custom tab from slack, there are no custom menu items from slack.
Assignee | ||
Comment 1•6 months ago
|
||
This is when the Incomplete Toolbar Redesign feature flag is active and the Custom Tab is opened from Slack.
Assignee | ||
Comment 2•6 months ago
|
||
This is when the "Incomplete Toolbar Redesign" feature flag is off.
Assignee | ||
Comment 3•6 months ago
|
||
How this bug was discovered:
It was pointed out that if a user opens a link in Slack on Android with the setting "open web pages in app", this will open a CustomTab inside of Slack with some Slack configured options.
Slack adds three menu items to the CustomTab menu: "Share via", "Copy Link", and "Share in Slack".
None of these three custom menu items in the CustomTab menu work. The buttons are no-ops.
We confirmed that this problem also exists if Chrome is the default browser app so the problem is with Slack and their CustomTabIntent
implementation.
So with this particular issue, it was determined that Slack has a bug with their Android app.
As part of investigating this issue, I determined that the Toolbar Redesign (navbar) menu does not even display these items from the CustomTabIntent
. This probably has to do with how the menu is now called from the bottom Toolbar (navbar) rather than the AddressBar toolbar.
One caveat: it seems that Instagram does still modify the CustomTab toolbar and does it successfully even with the "Incomplete Toolbar Redesign" feature flag active.
Comment 4•6 months ago
|
||
Tentatively tagging this bug as an experiment blocker.
Comment 5•6 months ago
•
|
||
Based on Harrison's evaluation above, this doesn't seem like a blocker to me:
- Slack has a bug with how they populate the Custom Tab menu (our original Toolbar and Chrome's toolbar both behave the same)
- Our new Toolbar successfully allows Instagram to populate the Custom Tab menu.
It's possible that this is actually "working as intended" - e.g., if our new Toolbar logic just (circumstantially) happens to filter our broken PendingIntents, perhaps that's why Slack's menu options aren't showing up in our redesign.
On the other hand, it's definitely still possible that we've broken some Custom Tab menu API, and that it needs to be fixed: but we need more information before we can make that assumption.
E.g.,
- If someone can find an app (other than Slack) that does lose Custom Tab menu options in the redesign, then that'd mean we've done something wrong for sure.
- If someone could debug the code path that populates the Custom Tab menu options in the redesign, perhaps you could spot the place where Slack's menu intents are getting omitted from our menu initialization.
All that said: I think it's OK to leave this as a blocker for now, but if we need to cut scope later, this could be a candidate until we determine more info based on either of the above ideas.
Comment 6•5 months ago
|
||
Harrison, does the Slack menu bug behaves in the same broken way whether the toolbar redesign's feature flag is enabled or not? Roger says fixing (or working around) the Slack menu bug doesn't necessarily block shipping the toolbar redesign, but we should make sure the toolbar redesign doesn't make the Slack menu bug worse.
Assignee | ||
Comment 7•5 months ago
|
||
The toolbar redesign doesn't make the Slack menu bug worse in my opinion since the menu items are broken regardless of what browser the user has. Other menu items are successfully being added to the custom tab menu with the redesign active.
I do think that it is worthwhile to keep an eye on this issue if Slack suddenly fixes their bug. (for context, it's now been a month since this issue was initially discovered and Slack has not fixed their bug.).
Assignee | ||
Updated•5 months ago
|
Assignee | ||
Comment 8•5 months ago
|
||
if the NavBar is enabled, the menu attached to it in the ExternalAppBrowserFragment doesn't go through any of the logic in CustomTabsToolbarFeature.
this is a bigger problem than just the Slack custom tab custom menu items.
Comment 9•5 months ago
|
||
I do think that it is worthwhile to keep an eye on this issue if Slack suddenly fixes their bug. (for context, it's now been a month since this issue was initially discovered and Slack has not fixed their bug.).
In that case, I'll leave this bug as beta blocker so we remember to check Slack before shipping phase 1.
Did we report this bug to Slack? The Webcompat team probably has contacts at Slack.
Assignee | ||
Comment 10•5 months ago
|
||
Updated•4 months ago
|
Comment 11•4 months ago
|
||
Increasing priority to P1 now that we're fixing toolbar phase 1's beta blockers.
Comment 12•3 months ago
|
||
Petru has looked into this work a bit, will determine if it should be re-assigned on unassigned while Harrison is out
Comment 13•3 months ago
|
||
Dropping priority to P2 because this bug is not in our current sprint, but still blocks our toolbar phase 1 beta milestone.
Comment 14•3 months ago
|
||
Comment 15•3 months ago
|
||
Working from the reported situation
a custom tab in Slack will show the menu items for "Share via", "Copy link", and "Share in Slack"
It seems to me like that has changed - there are no such menu items present anymore.
Also seeing in the attached Chrome Custom Tab recording that those buttons were unactionable even in Chrome so I'm thinking that that was actually a bug/not yet finished feature from Slack.
Testing now the same scenario in Chrome I see that there are no custom menu items.
And the same is happening in Firefox - see NoCustomMenuItemsForSlackInFirefox
Comment 16•3 months ago
|
||
When using the navbar indeed the menu was not populated with custom menu items
But that is now possible after bug 1904325 which was recently merged and verified as fixed by QA.
Closing this as WorksForMe.
Description
•