Closed
Bug 1122074
Opened 10 years ago
Closed 10 years ago
"Normal Tabs" tray has an empty state
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox36 verified, firefox37 verified, firefox38 verified, fennec-)
VERIFIED
FIXED
Firefox 38
People
(Reporter: antlam, Assigned: mhaigh)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Steps to reproduce
---
1) Go into Tabs tray
2) Swap to Private Tabs tray
3) Create a new Private Tab
4) Return to Private Tabs tray
5) Switch over to Normal Tabs tray
6) Last Normal Tab can be removed!
Device
---
Nexus 7, Kit Kat
Reporter | ||
Updated•10 years ago
|
Blocks: new-tablet-v1
Updated•10 years ago
|
tracking-fennec: --- → ?
Comment 1•10 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #0)
> 6) Last Normal Tab can be removed!
We allow this since it's not the last tab (you still have a private tab). If you want an empty state info graphic in the "normal tabs" tray, that's fine. Not tracking this though.
tracking-fennec: ? → -
Reporter | ||
Comment 2•10 years ago
|
||
We should just restore a normal tab then (just like on mobile). I want to avoid creating a normal tab tray empty state right now without carefully considering the entire picture cross device/platforms.
Updated•10 years ago
|
Assignee: nobody → mhaigh
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Make sure that there's always at least one open 'normal' tab open, else open one.
Attachment #8552489 -
Flags: review?(michael.l.comella)
Comment 4•10 years ago
|
||
Comment on attachment 8552489 [details] [diff] [review]
Normal Tabs tray has an empty state
Review of attachment 8552489 [details] [diff] [review]:
-----------------------------------------------------------------
lgtm, w/ fix for "only if removed, right?" or a good explanation.
::: mobile/android/base/tabs/TabsGridLayout.java
@@ +231,5 @@
> }
> +
> + final Tabs tabsInstance = Tabs.getInstance();
> +
> + if (tab.isPrivate() == mIsPrivate && mTabsAdapter.getCount() > 0) {
nit: We already make this check above - perhaps we should move this code block (sans redundant check) inside the if statement above.
@@ +239,5 @@
> + }
> + }
> +
> + // Make sure we always have at least one normal tab
> + if (!tab.isPrivate()) {
We should only do this if we successfully removed a tab, right?
@@ +241,5 @@
> +
> + // Make sure we always have at least one normal tab
> + if (!tab.isPrivate()) {
> + final Iterable<Tab> tabs = tabsInstance.getTabsInOrder();
> + boolean lastNormalTab = true;
nit: -> removedTabIsLastNormalTab
Attachment #8552489 -
Flags: review?(michael.l.comella) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Approval Request Comment
[Feature/regressing bug #]:
Prevent the new tablet tabs tray getting in to a state where a user has no open 'normal' tabs
[User impact if declined]:
The user could reach a state where they have no open 'normal' tabs
[Describe test coverage new/current, TreeHerder]:
Local testing
[Risks and why]:
The code touches parts of the new tablet tab panel close functionality - potentially we run the risk that closing tabs may not work.
[String/UUID change made/needed]:
N/A
Attachment #8553767 -
Flags: approval-mozilla-beta?
Attachment #8553767 -
Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Updated•10 years ago
|
Updated•10 years ago
|
Attachment #8553767 -
Flags: approval-mozilla-beta?
Attachment #8553767 -
Flags: approval-mozilla-beta+
Attachment #8553767 -
Flags: approval-mozilla-aurora?
Attachment #8553767 -
Flags: approval-mozilla-aurora+
Updated•10 years ago
|
Attachment #8552489 -
Attachment is obsolete: true
Comment 8•10 years ago
|
||
Comment 9•10 years ago
|
||
Verified as fixed in builds:
Firefox for Android 38.0a1 (2015-01-26)
Firefox for Android 37.0a2 (2015-01-26)
Device: Asus Transformer Pad TF300T (Android 4.2.1)
Comment 10•10 years ago
|
||
Verified as fixed in Firefox for Android 36 Beta 4;
Device: Asus Transformer Pad TF300T (Android 4.2.1).
Status: RESOLVED → VERIFIED
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•