Closed Bug 917944 Opened 6 years ago Closed 6 years ago

Remove "Most recent" header in history page

Categories

(Firefox for Android :: Awesomescreen, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 27
Tracking Status
firefox26 --- verified
firefox27 --- verified
fennec 26+ ---

People

(Reporter: Margaret, Assigned: capella)

References

Details

Attachments

(1 file, 2 obsolete files)

Now that we don't have "Most visited", the "Most recent" header doesn't feel necessary.

But we should keep the "Tabs from last time" header when that page has content.
(In reply to :Margaret Leibovic from comment #0)
> Now that we don't have "Most visited", the "Most recent" header doesn't feel
> necessary.
> 
> But we should keep the "Tabs from last time" header when that page has
> content.

That will make the tab switch feel a bit clunky, no? i.e. one tab with a title and the other without.
Maybe, maybe not. In our earlier History Panel, we had different lists for Most Frequent and Most Recent (this one had an extra header for date) and I never really noticed that...
Attached patch bug917944 (v0) (obsolete) — Splinter Review
Attachment #808291 - Flags: review?(margaret.leibovic)
This leaves the title field in the home_history_list.xml but doesn't populate it for the home_most_recent_page fragment ...
Status: NEW → ASSIGNED
QA Contact: markcapella
Comment on attachment 808291 [details] [diff] [review]
bug917944 (v0)

Review of attachment 808291 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

Note to self: we don't need to remove the string because it is also used to add the tab for this page in HistoryPage.java.
Attachment #808291 - Flags: review?(margaret.leibovic) → review+
This is a small polish issue we'll probably want to uplift to 26 with the new-new-about-home changes.
tracking-fennec: --- → ?
Assignee: nobody → markcapella
QA Contact: markcapella
And on we go ... I accept chocolate bit-coins
https://hg.mozilla.org/integration/fx-team/rev/403743a219a3
mmm ... I passed a try push ... testShareLink mid-air collision?
Attached patch bug917944 (v1) (obsolete) — Splinter Review
Switching the waitForText() value from the "Most recent" literal to "Today" works out just fine ...

https://tbpl.mozilla.org/?tree=Try&rev=94df3d872089
Attachment #808291 - Attachment is obsolete: true
Attachment #809760 - Flags: review?(margaret.leibovic)
Comment on attachment 809760 [details] [diff] [review]
bug917944 (v1)

Review of attachment 809760 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/tests/StringHelper.java.in
@@ +92,5 @@
>      public static final String HISTORY_LABEL = "HISTORY";
>      public static final String TOP_SITES_LABEL = "TOP SITES";
>      public static final String BOOKMARKS_LABEL = "BOOKMARKS";
>      public static final String READING_LIST_LABEL = "READING LIST";
> +    public static final String MOST_RECENT_LABEL = "Today";

I would prefer to see you rename this variable TODAY_LABEL, then update where it's used, adding a comment explaining this assumption that pages must be visited during the test run for this check to pass.
Attachment #809760 - Flags: review?(margaret.leibovic) → review-
Attached patch bug917944 (v2)Splinter Review
Nice touches.
Attachment #809760 - Attachment is obsolete: true
Attachment #810124 - Flags: review?(margaret.leibovic)
Comment on attachment 810124 [details] [diff] [review]
bug917944 (v2)

Thanks.
Attachment #810124 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/1c5e6b8ebbde
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
tracking-fennec: ? → 26+
Capella, request uplift to Aurora?
Flags: needinfo?(markcapella)
Comment on attachment 810124 [details] [diff] [review]
bug917944 (v2)

[Approval Request Comment] This has been in m-c for about three weeks, fairly easy bug ... not sure who q/a'd it ... looks like resolved "fixed" by Tomcat

Bug caused by (feature/regressing bug #): Initial About:home implementation
User impact if declined: They can wait a little longer for it to get prettier.
Testing completed (on m-c, etc.): local to me, m-c/nightly by me
Risk to taking this patch (and alternatives if risky): It might interrupt someones work flow? meh - none - we can put if back if we have to.
String or IDL/UUID changes made by this patch: None
Attachment #810124 - Flags: approval-mozilla-aurora?
Flags: needinfo?(markcapella)
Attachment #810124 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on:
Build: Firefox for Android 26.0a2 (2013-10-28) and Firefox for Android 27.0a1 (2013-10-28)
Device: Samsung Galaxy Nexus
OS: Android 4.1.1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.