Closed
Bug 844816
Opened 12 years ago
Closed 12 years ago
Awesome-screen tab text becomes indiscernible after font color changes from opening a private tab via context menu
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox20 affected, firefox21 affected, firefox22 verified, fennec21+)
VERIFIED
FIXED
Firefox 22
People
(Reporter: paul.feher, Assigned: sriram)
References
Details
(Keywords: reproducible)
Attachments
(5 files, 1 obsolete file)
43.95 KB,
image/png
|
Details | |
3.08 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
2.66 KB,
patch
|
mfinkle
:
review-
|
Details | Diff | Splinter Review |
17.36 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
7.57 KB,
patch
|
bnicholson
:
review+
|
Details | Diff | Splinter Review |
Aurora 21.0a2 2012-02-24
Device: Samsung Galaxy R (Android 2.3.4) , LG Slider ARMv6 (Android 2.3.4)
Steps to reproduce:
1. Open awesome screen.
2. Long tap on any link listen in Top Sites for the context menu tu appear.
3. Select open in private tab option.
4. Click on History section.
Expected results:
The awesome screen sections should keep the same font colour.
Actual results:
The awesome screen sections become white after opening a private tab from context menu. They are impossible to be read because the background remains also white.
Comment 1•12 years ago
|
||
Is Fx20 affected?
tracking-fennec: --- → ?
Keywords: reproducible
Summary: Awesome screen sections become white after opening a private tab from context menu → Awesome-screen tab text becomes discernable after font color changes from opening a private tab via context menu
Reporter | ||
Updated•12 years ago
|
status-firefox20:
--- → affected
status-firefox22:
--- → affected
Summary: Awesome-screen tab text becomes discernable after font color changes from opening a private tab via context menu → Awesome-screen tab text becomes indiscernible after font color changes from opening a private tab via context menu
Comment 2•12 years ago
|
||
Yeah reproducible on my Nexus 4
Comment 3•12 years ago
|
||
Paul would you retest with a build that has the fix for 845572 and see if this still happens?
Assignee: nobody → sriram
tracking-fennec: ? → 20+
Flags: needinfo?(paul.feher)
Comment 4•12 years ago
|
||
This is also an issue on Firefox 20 beta 2 build 3 if you open a normal tab from the context menu in the private tab awesomesceen on the Acer A500 (Android 3.2.1)
Reporter | ||
Comment 5•12 years ago
|
||
Yes this is still reproducible on latest Nightly 22.0a1 2012-03-01 using the same device.
Flags: needinfo?(paul.feher)
Updated•12 years ago
|
Flags: needinfo?(sriram)
Comment 6•12 years ago
|
||
Screenshot.
Still reproducible on trunk and all channels.
Updated•12 years ago
|
Status: NEW → ASSIGNED
Updated•12 years ago
|
tracking-fennec: 20+ → 21+
Assignee | ||
Comment 7•12 years ago
|
||
Menu items are removed from the XML file. (Strings as a separate patch).
Attachment #727875 -
Flags: review?(mark.finkle)
Flags: needinfo?(sriram)
Assignee | ||
Comment 8•12 years ago
|
||
This patch removes the strings (release-cycle friendly :D ).
Attachment #727877 -
Flags: review?(mark.finkle)
Comment 9•12 years ago
|
||
Comment on attachment 727875 [details] [diff] [review]
Part 1: Remove menu items
This can only land if we also add these action to the about:home thumbnails
Attachment #727875 -
Flags: review?(mark.finkle) → review+
Comment 10•12 years ago
|
||
Comment on attachment 727877 [details] [diff] [review]
Part 2: Remove strings
We'll need the strings for the about:home actions
Attachment #727877 -
Flags: review?(mark.finkle) → review-
Assignee | ||
Comment 11•12 years ago
|
||
Moved the menu items to about:home.
1. The way the AboutHomeContent handled ContextMenu was wrong. The selected position is available in AdapterView.AdapterContextMenuInfo by default. That (could) should be used as its free. Using an mSelected to store and use it is unnecessary. This caused some cleanup in the code.
2. Added a LOADURL_BACKGROUND flag. I thought of adding a LOADURL_SELECTED flag. But then, this is the default case and none would be passing this. Hence, the exception background case has a flag (just like delay load).
3. A small change in browser.js as the way we processed "selected" was incomplete.
Attachment #729620 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 12•12 years ago
|
||
Accidentally added:
//selectTab(added.getId());
I could remove this (or the entire block, as the browser.js handles it anyways).
Comment 13•12 years ago
|
||
Comment on attachment 729620 [details] [diff] [review]
Part 3: about:home
>diff --git a/mobile/android/base/Tabs.java b/mobile/android/base/Tabs.java
>+ args.put("selected", !background);
I am worried about changing this now. We could file a new bug to handle this, if we even want to do it.
>- if ((added != null) && !delayLoad) {
>- selectTab(added.getId());
>+ if ((added != null) && !delayLoad && !background) {
>+ //selectTab(added.getId());
Keep the "selectTab" in place for now. We need it for time when Gecko (JS) is not ready yet... primarily during startup.
>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js
> let params = {
>- selected: !delayLoad,
>+ selected: ("selected" in data) ? data.selected : !delayLoad,
Let's move this to a new bug (see above) if we want it at all
r+, but revert the parts I called out.
Attachment #729620 -
Flags: review?(mark.finkle) → review+
Comment 14•12 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #13)
> Comment on attachment 729620 [details] [diff] [review]
> Part 3: about:home
>
> >diff --git a/mobile/android/base/Tabs.java b/mobile/android/base/Tabs.java
>
> >+ args.put("selected", !background);
>
> I am worried about changing this now. We could file a new bug to handle
> this, if we even want to do it.
>
> >- if ((added != null) && !delayLoad) {
> >- selectTab(added.getId());
> >+ if ((added != null) && !delayLoad && !background) {
> >+ //selectTab(added.getId());
>
> Keep the "selectTab" in place for now. We need it for time when Gecko (JS)
> is not ready yet... primarily during startup.
>
> >diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js
>
> > let params = {
> >- selected: !delayLoad,
> >+ selected: ("selected" in data) ? data.selected : !delayLoad,
>
> Let's move this to a new bug (see above) if we want it at all
>
> r+, but revert the parts I called out.
Talked to Brian and Sriram on IRC. It seems we have no way to open a tab in the background from Java. We'll need _all_ of this patch. Make it so!
Assignee | ||
Comment 15•12 years ago
|
||
Comment 16•12 years ago
|
||
Assignee | ||
Comment 17•12 years ago
|
||
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=9ce01b2e6091
Assignee | ||
Comment 18•12 years ago
|
||
This doesn't search for the "Open in new tab" menu item.
Attachment #730970 -
Flags: review?(bnicholson)
Comment 19•12 years ago
|
||
Comment on attachment 730970 [details] [diff] [review]
Patch: Remove tests in AwesomeBar
Looks good to me, but why abort the try build?
Attachment #730970 -
Flags: review?(bnicholson) → review+
Assignee | ||
Comment 20•12 years ago
|
||
Try with new changes: https://tbpl.mozilla.org/?tree=Try&rev=c6820073dfce
Assignee | ||
Comment 21•12 years ago
|
||
This removes everywhere.
Attachment #730970 -
Attachment is obsolete: true
Attachment #731048 -
Flags: review?(bnicholson)
Updated•12 years ago
|
Attachment #731048 -
Flags: review?(bnicholson) → review+
Assignee | ||
Comment 22•12 years ago
|
||
Comment 23•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c710854a413f
https://hg.mozilla.org/mozilla-central/rev/36eca1fb1c5c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Updated•12 years ago
|
Comment 24•11 years ago
|
||
Verified fix on Firefox Mobile 22 beta 3 on the Asus EEE Transformer TF101 (Android 4.0.4). Removing the options from the context menu prevents the user from hitting the state described in Comment 0.
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
•