Closed
Bug 1002303
Opened 11 years ago
Closed 10 years ago
Provide a description on private tabs page if there are no private tabs
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 33
People
(Reporter: wesj, Assigned: mcomella)
References
Details
Attachments
(9 files, 7 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
antlam
:
feedback+
|
Details |
(deleted),
image/png
|
antlam
:
feedback+
|
Details |
(deleted),
image/png
|
antlam
:
feedback+
|
Details |
(deleted),
image/png
|
antlam
:
feedback+
|
Details |
(deleted),
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mcomella
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
eedens
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•11 years ago
|
Blocks: contextual-hints
Comment 1•10 years ago
|
||
Ooh, good idea. There's a plan in place to show things in the Synced Tabs tray, and great progress in place: https://bugzilla.mozilla.org/show_bug.cgi?id=958889.
Comment 2•10 years ago
|
||
This bug isn't quite ready for action, but I'm interested in mentoring this. We'll need to get UX feedback to get a visual mockup to see what the description would look like; and we should wait for Bug 958889 to land, since that's going to provide a model for how to implement this "panel that sometimes looks very different".
Whiteboard: [mentor=nalexander][lang=java][good second bug]
Comment 3•10 years ago
|
||
Since this is actionable, I'd like UX to take a crack at a mockup and have someone start on it ASAP.
Ian, Anthony: Can one of you take a crack.
Flags: needinfo?(ibarlow)
Flags: needinfo?(alam)
Assignee | ||
Updated•10 years ago
|
OS: Windows 7 → Android
Hardware: x86_64 → All
Comment 4•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #1)
> Ooh, good idea. There's a plan in place to show things in the Synced Tabs
> tray, and great progress in place:
> https://bugzilla.mozilla.org/show_bug.cgi?id=958889.
To Nick's point, I would definitely like to use a similar layout for the private tab (when it's empty). I'll mock something up for you guys but I want to ask what the "goal" of this page would be? so that I can write appropriate content.
Flags: needinfo?(ibarlow)
Reporter | ||
Comment 5•10 years ago
|
||
At the very least, just a simple "Your private tabs show up here" message to provide a hint what this is. I wondered for a bit if we'd be better to put the data from about:privatebrowsing in here (and show Top Sites like normal in private mode instead of showing about:privatebrowsing?). i.e. it reads something like:
====
Private Browsing
Firefox won't keep any browser history, search history, download history, web form history, cookies, or temporary internet files for tabs in here. However, files you download and bookmarks you make will be kept.
While this computer won't have a record of your browsing history, your internet service provider or employer can still track the pages you visit.
====
it also has a "Learn more" link. That interaction likely won't work well for people who create private tabs other ways though and never see the empty tabs tray.
Reporter | ||
Comment 6•10 years ago
|
||
Desktop also has a nifty about:privatebrowsing page if you open in in a non-private window:
=====
Would you like to start Private Browsing?
You are not currently in a private window.
In a Private Browsing window, Nightly won't keep any browser history, search history, download history, web form history, cookies, or temporary internet files. However, files you download and bookmarks you make will be kept.
To start Private Browsing, you can also select New Private Window from the menu.
=====
Comment 7•10 years ago
|
||
(In reply to Anthony Lam from comment #4)
> To Nick's point, I would definitely like to use a similar layout for the
> private tab (when it's empty). I'll mock something up for you guys but I
> want to ask what the "goal" of this page would be? so that I can write
> appropriate content.
I think the main goal is to let people know what Private Tabs can do and how they block history. We have some text in about:privatebrowsing that might be useful.
Comment 8•10 years ago
|
||
Here is a mock for content! It would ideally just follow the same style template for text that we are using on the sync side.
Note: The 'add a new tab' icon is different to what we have but it's not final at all, i just included it to see what it would look like.
I also want to point out that we have this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=864958
Flags: needinfo?(alam)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Whiteboard: [mentor=nalexander][lang=java][good second bug]
Assignee | ||
Comment 9•10 years ago
|
||
Note, in particular, my changes to TabsPanelItem.Button.
Attachment #8432893 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8432901 -
Flags: feedback?(alam)
Assignee | ||
Comment 11•10 years ago
|
||
Same styles as sync.
Removed the header because there wasn't much room.
Attachment #8432903 -
Flags: feedback?(alam)
Comment 12•10 years ago
|
||
Comment on attachment 8432901 [details]
Phone portrait
Looking good!
Attachment #8432901 -
Flags: feedback?(alam) → feedback+
Comment 13•10 years ago
|
||
Comment on attachment 8432903 [details]
Phone landscape
This looks a bit off. The line-height of the copy on the left definitely needs to be increased. Could we also try vertically raising the "Want to learn more?" link to be more centered?
I would also like to move the "active" tab a bit further down to open up these horizontal screens but I feel like that is going to open up a nest of other problems.
Attachment #8432903 -
Flags: feedback?(alam) → feedback-
Comment 14•10 years ago
|
||
Comment on attachment 8432893 [details] [diff] [review]
Part 1: Rename RemoteTabs* styles to be used with all tabs panels.
Review of attachment 8432893 [details] [diff] [review]:
-----------------------------------------------------------------
Nice.
Attachment #8432893 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 15•10 years ago
|
||
Design questions:
- Isn't this empty state a bit too verbose? Maybe the text should be more concise?
- It would be nice if we replaced the longer description with a simpler empty state once the user has created and closed a few private tabs. Seeing this description all the time might get a bit annoying. Maybe in a follow-up?
Comment 16•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #15)
> Design questions:
> - Isn't this empty state a bit too verbose? Maybe the text should be more
> concise?
> - It would be nice if we replaced the longer description with a simpler
> empty state once the user has created and closed a few private tabs. Seeing
> this description all the time might get a bit annoying. Maybe in a follow-up?
I agree - I would like to follow up these visual inconsistencies and establish a more solid design framework in a later bug.
Assignee | ||
Comment 17•10 years ago
|
||
Attachment #8433692 -
Flags: feedback?(alam)
Assignee | ||
Comment 18•10 years ago
|
||
Attachment #8433693 -
Flags: feedback?(alam)
Assignee | ||
Comment 19•10 years ago
|
||
Attachment #8433696 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 20•10 years ago
|
||
Comment on attachment 8433696 [details] [diff] [review]
Part 0: Add empty private browsing view description strings.
Hey, Ryan. Anthony said you might know where to point the "Want to learn more" link in his mock from comment 8 - where should it link the user to?
Clearing review until this link is updated.
Attachment #8433696 -
Flags: review?(lucasr.at.mozilla)
Flags: needinfo?(rfeeley)
Assignee | ||
Comment 21•10 years ago
|
||
WIP.
Assignee | ||
Comment 22•10 years ago
|
||
Comment on attachment 8433722 [details] [diff] [review]
Part 2: Show a description page when there are no private tabs.
These styles are getting stupidly messy because I'm trying to make them inherit where they perhaps should not be inheriting.
Assuming antlam likes my tablet design, TabsPanelItem.TextAppeareance.Header.HiddenInLandscape.PrivateTabs is ambiguous - on phone, we hide the title in landscape, not in portrait. On tablet, it's the reverse! I don't know how to make this obvious without "...HiddenInLandscape.ReversedOnTablet", which seems really dumb. Alteratively, don't make it an inheritance (i.e. remove the ".").
Similiar issues are abound with the 2ColCentered layout, and anything appended with "PrivateTabs". Do you have any suggestions?
Attachment #8433722 -
Flags: feedback?(lucasr.at.mozilla)
Comment 23•10 years ago
|
||
Comment on attachment 8433722 [details] [diff] [review]
Part 2: Show a description page when there are no private tabs.
Review of attachment 8433722 [details] [diff] [review]:
-----------------------------------------------------------------
Looks generally good but yes, try to avoid all these style inheritances as they are not easy to follow and likely to be broken by the next person changing them due all the dependencies.
Attachment #8433722 -
Flags: feedback?(lucasr.at.mozilla) → feedback+
Assignee | ||
Comment 24•10 years ago
|
||
Removed inheritance as much as I felt reasonable, as per comment 23.
Attachment #8435370 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8433722 -
Attachment is obsolete: true
Assignee | ||
Comment 25•10 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #13)
> Comment on attachment 8432903 [details]
> Phone landscape
>
> This looks a bit off. The line-height of the copy on the left definitely
> needs to be increased.
I didn't touch this because it may get stomped on with l10n and small screen sizes. I filed bug 1021356 for the visual refinements.
> Could we also try vertically raising the "Want to
> learn more?" link to be more centered?
Done.
Attachment #8432903 -
Attachment is obsolete: true
Attachment #8435375 -
Flags: feedback?(alam)
Comment 26•10 years ago
|
||
Comment on attachment 8435375 [details]
Phone landscape v2
This is looking better
Attachment #8435375 -
Flags: feedback?(alam) → feedback+
Updated•10 years ago
|
Attachment #8433693 -
Flags: feedback?(alam) → feedback+
Comment 27•10 years ago
|
||
Comment on attachment 8435370 [details] [diff] [review]
Part 2: Show a description page when there are no private tabs.
Review of attachment 8435370 [details] [diff] [review]:
-----------------------------------------------------------------
Looking good, just needs to change the structure of PrivateTabsPanel a bit.
::: mobile/android/base/resources/layout/private_tabs_panel.xml
@@ +12,5 @@
> + android:visibility="gone">
> +
> + <org.mozilla.gecko.tabspanel.TabsTray android:id="@+id/private_tabs_tray"
> + style="@style/TabsList"
> + android:layout_width="fill_parent"
fill_parent -> match_parent for consistency?
::: mobile/android/base/tabspanel/PrivateTabsPanel.java
@@ +28,5 @@
> + super(context, attrs);
> + }
> +
> + @Override
> + public void onFinishInflate() {
This composite view looks good overall but I'd rather avoid relying on onFinishInflate() as I've seen device-specific bugs where the inflation callbacks are not very reliable (e.g. callbacks called more than once, not called at all, etc). So, let's tweak the structure of this view to be more like, say, TwoLinePageRow:
1. Inflate the inner layout in the constructor (change private_tabs_panel.xml to have a 'merge' toplevel tag)
2. Initialize view members in the constructor
3. Use view tag (org.mozilla.gecko.tabspanel.PrivateTabsPanel) directly in tabs_panel.xml instead of using an include
Attachment #8435370 -
Flags: review?(lucasr.at.mozilla) → feedback+
Comment 28•10 years ago
|
||
Comment on attachment 8433692 [details]
Tablet portrait
I think this is good for now.. as mentioned before, we're probably still going to have to take a look at all these trays at a later date.
Attachment #8433692 -
Flags: feedback?(alam) → feedback+
Updated•10 years ago
|
Flags: needinfo?(rfeeley)
Assignee | ||
Comment 29•10 years ago
|
||
Hey, Ryan. Anthony said you might know where to point the "Want to learn more" link in his mock from comment 8 - where should it link the user to?
Flags: needinfo?(rfeeley)
Assignee | ||
Comment 30•10 years ago
|
||
Updated for lucas' comments.
Attachment #8439607 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8435370 -
Attachment is obsolete: true
Comment 31•10 years ago
|
||
Comment on attachment 8439607 [details] [diff] [review]
Part 2: Show a description page when there are no private tabs.
Review of attachment 8439607 [details] [diff] [review]:
-----------------------------------------------------------------
Nice, thanks.
::: mobile/android/base/resources/layout-large-land-v11/tabs_panel.xml
@@ -39,5 @@
> android:choiceMode="singleChoice"
> android:visibility="gone"
> gecko:tabs="tabs_normal"/>
>
> - <org.mozilla.gecko.tabspanel.TabsTray android:id="@+id/private_tabs"
It seems you forgot to update this file.
Attachment #8439607 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 32•10 years ago
|
||
Got the link from Ian via email.
Attachment #8440175 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8433696 -
Attachment is obsolete: true
Assignee | ||
Comment 33•10 years ago
|
||
Updated the patch to fill in the locale of the link.
Attachment #8440178 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8439607 -
Attachment is obsolete: true
Updated•10 years ago
|
Attachment #8440175 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 34•10 years ago
|
||
Comment on attachment 8440178 [details] [diff] [review]
Part 2: Show a description page when there are no private tabs.
Review of attachment 8440178 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good with the fix in layout-large-land-v11/tabs_panel.xml
::: mobile/android/base/resources/layout-large-land-v11/tabs_panel.xml
@@ +39,5 @@
> android:choiceMode="singleChoice"
> android:visibility="gone"
> gecko:tabs="tabs_normal"/>
>
> + <include layout="@layout/private_tabs_panel"/>
This should be <org.mozilla.gecko.tabspanel.PrivateTabsPanel ...> directly here instead of the include.
Attachment #8440178 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 35•10 years ago
|
||
Updated for nits.
Assignee | ||
Updated•10 years ago
|
Attachment #8440178 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8440957 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(rfeeley)
Assignee | ||
Comment 36•10 years ago
|
||
Comment 37•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c69d0ec8f5de
https://hg.mozilla.org/mozilla-central/rev/e8993855b30f
https://hg.mozilla.org/mozilla-central/rev/1a18c37601b7
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Comment 38•10 years ago
|
||
>+<!ENTITY private_tabs_panel_description "Your private tabs will show up here. While we don\'t keep any form of your browsing history or cookies, files that you download will still be saved on your device.">
Files that you download, and bookmarks you make will be kept. Also, I suggest dropping 'form' to not mix it up with 'form history'.
Assignee | ||
Comment 39•10 years ago
|
||
"Your private tabs will show up here. While we don\'t keep any of your browsing history or cookies, bookmarks and any files that you download will still be saved on your device."
Anthony, what do you think of the adjustments I made to the issues brought up in comment 38?
Flags: needinfo?(alam)
Comment 40•10 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #39)
> "Your private tabs will show up here. While we don\'t keep any of your
> browsing history or cookies, bookmarks and any files that you download will
> still be saved on your device."
>
> Anthony, what do you think of the adjustments I made to the issues brought
> up in comment 38?
"Your private tabs will show up here. While we don't keep any of your browsing history or cookies, bookmarks and files that you download will still be saved on your device."
I removed the "any" because I don't really think that's necessary :)
Flags: needinfo?(alam)
Assignee | ||
Comment 41•10 years ago
|
||
Attachment #8443651 -
Flags: review?(wjohnston)
Assignee | ||
Comment 42•10 years ago
|
||
Comment on attachment 8443651 [details] [diff] [review]
Part 3: Update empty private tabs panel description.
Review of attachment 8443651 [details] [diff] [review]:
-----------------------------------------------------------------
Actually... :)
See the above comments on the reasons for this change.
Attachment #8443651 -
Flags: review?(wjohnston) → review?(eedens)
Assignee | ||
Updated•10 years ago
|
Attachment #8443651 -
Attachment is obsolete: true
Attachment #8443651 -
Flags: review?(eedens)
Comment 44•10 years ago
|
||
Comment on attachment 8443688 [details] [diff] [review]
Part 3: Update empty private tabs panel description.
Review of attachment 8443688 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me -- text matches the text from antlam's previous comment
Attachment #8443688 -
Flags: review?(eedens) → review+
Assignee | ||
Comment 45•10 years ago
|
||
Nice catch, Aaron: https://hg.mozilla.org/integration/fx-team/rev/0f335eff972f
Comment 46•10 years ago
|
||
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
•