Closed
Bug 880230
Opened 12 years ago
Closed 11 years ago
Quick share item swaps between top item despite never selecting it
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox22 unaffected, firefox23 unaffected, firefox24 affected, firefox25 affected, firefox26 affected, firefox27 affected, firefox28 verified, fennec28+)
VERIFIED
FIXED
Firefox 28
Tracking | Status | |
---|---|---|
firefox22 | --- | unaffected |
firefox23 | --- | unaffected |
firefox24 | --- | affected |
firefox25 | --- | affected |
firefox26 | --- | affected |
firefox27 | --- | affected |
firefox28 | --- | verified |
fennec | 28+ | --- |
People
(Reporter: aaronmt, Assigned: wesj)
References
Details
Attachments
(2 files, 1 obsolete file)
2.02 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
2.62 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
Currently when one always selects the same item in the share menu, the wrong item will be displayed in quick-share.
STR:
i) Open the share menu and select 'copy to clipboard'
ii) Repeat the first step a few times
Expected: 'Copy to clipboard is quick-shared
Actual: Different item is quick-shared, in my case, Google Drive
--
HTC One (Android 4.1)
Nightly (06/06)
Reporter | ||
Comment 1•12 years ago
|
||
Each time afterwards that I select 'Copy to clipboard' as an item, it swaps back between the top-most item, in my case, Drive, and 'Copy to clipboard'.
Reporter | ||
Comment 2•12 years ago
|
||
Here's a video because you're probably confused: http://www.youtube.com/watch?v=jnrrIAJDyQc
Comment 3•12 years ago
|
||
I'm playing with this ... as usual, if someone needs to jump in faster, please let me know.
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Reporter | ||
Updated•12 years ago
|
status-firefox22:
--- → unaffected
status-firefox23:
--- → unaffected
status-firefox24:
--- → affected
Reporter | ||
Comment 4•12 years ago
|
||
Confirmed that this happens with Google Drive installed (or maybe any package that adds additional activities). Installing Google Drive [1], adds a 'send text to clipboard activity'.
[1] com.google.android.apps.docs.shareitem.UploadSharedItemActivity
[2] com.google.android.apps.docs.SendTextToClipboardActivity
Uninstalling Google Drive removes both activities.
I think the problem here is logic getting confused on which activity to pick when multiple are available under a common package like com.google.android.apps.docs.*
Comment 5•11 years ago
|
||
Releasing since I haven't looked at this in a while
Assignee: markcapella → nobody
Status: ASSIGNED → NEW
Reporter | ||
Updated•11 years ago
|
status-firefox25:
--- → affected
status-firefox26:
--- → affected
Reporter | ||
Comment 6•11 years ago
|
||
This is still a complete nuisance.
Updated•11 years ago
|
Assignee: nobody → sriram
tracking-fennec: ? → 28+
Assignee | ||
Comment 7•11 years ago
|
||
Taking this. Aaron was right. We're using the packageName to compare these. For packages that provide multiple intents (with different classes handling them) that leads to confusion.
Assignee: sriram → wjohnston
Assignee | ||
Comment 8•11 years ago
|
||
Two fixes. This fixes the count returned from the Model. This is used to determine if we show one or two items in the menu. Since the db holds a record for each time share was used, using it twice with the same service was causing this to return 2 before. This fixes it to only count services with unique names.
Attachment #8337118 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 9•11 years ago
|
||
This fixes the real bug here. Uses the un-ambiguous packageName/class in this map of activities so that Drive and Copy to clipboard don't get thrown together into the same key.
Attachment #8337119 -
Flags: review?(lucasr.at.mozilla)
Comment 10•11 years ago
|
||
Comment on attachment 8337118 [details] [diff] [review]
Patch 1/2
Review of attachment 8337118 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/base/widget/ActivityChooserModel.java
@@ +663,5 @@
> */
> public int getHistorySize() {
> synchronized (mInstanceLock) {
> ensureConsistentState();
> + List<String> packages = new ArrayList<String>();
nit: final
@@ +669,5 @@
> + String activity = record.activity.flattenToString();
> + if (!packages.contains(activity)) {
> + packages.add(activity);
> + }
> + }
This will change the semantics of the existing API which might lead to confusion later. The notion of 'history' in the current API is about tracking every single time the user has chosen an activity to share something. Which in turn is different than things like getActivityCount().
It seems our UI needs something like the number of unique activities in history. So I'd probably add a new method like getActivityCountInHistory() or something.
Attachment #8337118 -
Flags: review?(lucasr.at.mozilla) → review-
Comment 11•11 years ago
|
||
Comment on attachment 8337119 [details] [diff] [review]
Patch 2/2
Review of attachment 8337119 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/base/widget/ActivityChooserModel.java
@@ +980,5 @@
> final int lastShareIndex = historicalRecords.size() - 1;
> float nextRecordWeight = 1;
> for (int i = lastShareIndex; i >= 0; i--) {
> HistoricalRecord historicalRecord = historicalRecords.get(i);
> + String packageName = historicalRecord.activity.flattenToString();
Maybe file a bug in upstream Android as well? They'll probably ignore it but, hey, we're doing our part ;-)
Attachment #8337119 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 12•11 years ago
|
||
Assignee | ||
Comment 13•11 years ago
|
||
Good idea.
Attachment #8337118 -
Attachment is obsolete: true
Attachment #8337849 -
Flags: review?(lucasr.at.mozilla)
Comment 14•11 years ago
|
||
Comment on attachment 8337849 [details] [diff] [review]
Patch 1/2
Review of attachment 8337849 [details] [diff] [review]:
-----------------------------------------------------------------
Nice.
::: mobile/android/base/widget/ActivityChooserModel.java
@@ +667,5 @@
> return mHistoricalRecords.size();
> }
> }
>
> + public int getNumUniqueActivitiesInHistory() {
The reason why I suggested getActivityCountInHistory() is because is that it would match well with the existing getActivityCount() method. I don't feel strongly about it. Up to you.
Attachment #8337849 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 15•11 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #14)
> > + public int getNumUniqueActivitiesInHistory() {
>
> The reason why I suggested getActivityCountInHistory() is because is that it
> would match well with the existing getActivityCount() method. I don't feel
> strongly about it. Up to you.
My OCD likes getActivityCountInHistory or getDistinctActivityCountInHistory
Comment 16•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Updated•11 years ago
|
Reporter | ||
Updated•11 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
•