Closed Bug 745225 Opened 9 years ago Closed 9 years ago

No highlighting when clicking a synced tab link on About:home

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 16
Tracking Status
firefox15 --- verified
blocking-fennec1.0 --- -

People

(Reporter: tchung, Assigned: sriram)

Details

(Keywords: polish)

Attachments

(3 files)

on About:home page, the links from "synced tabs" listed at the bottom, do not get highlighted when tapping on them to launch.   But, they are highlighted just fine in the section above, "Your tabs from last time"

Nom'ing for triage, im thinking soft with low-risk fix?

Repro:
1) 4-12-2012 nightly, Galaxy Nexus (Andr 4.0.2), and HTC Sensation (Andr 2.3.4)
2) setup sync to desktop, and sync some tabs
3) goto about:home, and scroll to bottom where Synced tabs section is
4) tap a link there.  verify no highlighting of link.

Expected:
- Highlight synced tabs link just like how we highlight other links on this page

Actual:
- synced tabs link dont highlight.
Summary: No highlighting when clicking a synced tab on About:home → No highlighting when clicking a synced tab link on About:home
CC'ing Sriram
Keywords: polish
This doesnt happen for any row in about:home.
blocking-fennec1.0: ? → soft
All the rows in about:home used a 9 png background: https://hg.mozilla.org/mozilla-central/raw-file/c61e7c3a232a/mobile/android/base/resources/drawable/abouthome_separator.9.png

This 9 png has the "2dp" divider to be shown above the row, and a transparent region that doesnt affect the row. This is not a "state-level" drawable to show a highlight on the row. Also, this cannot be done with this resource.

Reasons:
* The system highlight color cannot be "added" with the divider on the fly. So, the divider and the highlight/normal drawables should be separated.
* If the divider is separated, it should have it's own view. This needs to be added with every row added (Either as a part of the row, or as a part of the parent).

This is a bigger change and affects the layout of entire about:home. Probably this can be taken up only after 1.0 cut-off so that it doesnt affect anything much.

(On a side note, I have a patch that tries to do this -- half way through -- and there is a real good performance improvement).

Dirty fix: State-level drawable with normal state as it is now, and highlight state from the system. The divider won't be visible on highlight. And there will be some sort of visual glitch due to the divider vanishing on press.
blocking-fennec1.0: soft → ?
blocking-fennec1.0: ? → -
Attached patch PatchSplinter Review
This cleanup the about:home content. The dividers are not with the background, but added separately. Hence each section will have its own dividers (add to each row / more text / title / subtitle).

This is a prerequisite to re-arrange sections based on need for tablets.
Attachment #641323 - Flags: review?(mark.finkle)
Comment on attachment 641323 [details] [diff] [review]
Patch

Do we need to add the new views to the view factory for performance?
I notice we put a lot of colors in the layout XML. Should those be added to color.xml ?
Attachment #641323 - Flags: review?(mark.finkle) → review+
I thought of doing that. We currently do that optimization for static inner classes. I'll do that as a followup.

Also, I am thinking of renaming all "org.mozilla.gecko.Something" to "Gecko.Something" in XML, its simple and short. Now that we are supplying the required class, we can rename the XML tag to anything of our likes.
This patch optimizes static inner classes.
Attachment #641636 - Flags: review?(mark.finkle)
Attached patch Patch 3: RenameSplinter Review
This patch renames our custom views to "Gecko.Something". This is more readable and ensures a good XML coding pattern.

1. Any custom view should be Gecko.Something.
2. "Something" should be added to GeckoViewsFactory.
Attachment #641647 - Flags: review?(mark.finkle)
https://hg.mozilla.org/mozilla-central/rev/11900afd18de
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee: nobody → sriram
Target Milestone: --- → Firefox 16
Comment on attachment 641323 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: No highlighting on about:home.
Testing completed (on m-c, etc.): Landed on m-c on 07/13
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #641323 - Flags: approval-mozilla-beta?
Attachment #641323 - Flags: approval-mozilla-aurora?
Comment on attachment 641323 [details] [diff] [review]
Patch

no risk patch, approved.
Attachment #641323 - Flags: approval-mozilla-beta?
Attachment #641323 - Flags: approval-mozilla-beta+
Attachment #641323 - Flags: approval-mozilla-aurora?
Attachment #641323 - Flags: approval-mozilla-aurora+
This patch has been come to aurora after merge.
Status: RESOLVED → VERIFIED
Comment on attachment 641636 [details] [diff] [review]
Patch 2: Custom view optimizations

File a new bug if we still want this patch
Attachment #641636 - Flags: review?(mark.finkle)
Comment on attachment 641647 [details] [diff] [review]
Patch 3: Rename

File a new bug if you still want this patch
Attachment #641647 - Flags: review?(mark.finkle)
You need to log in before you can comment on or make changes to this bug.