Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Don't show the list of activities if there is only one

RESOLVED FIXED in Firefox 13

Status

()

Core
Widget: Android
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla14
All
Android
Points:
---

Firefox Tracking Flags

(firefox13 fixed, firefox14 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 604404 [details] [diff] [review]
Patch v1

Follow-up from bug 730289. Quite simple actually.
Attachment #604404 - Flags: review?(doug.turner)

Comment 1

5 years ago
Comment on attachment 604404 [details] [diff] [review]
Patch v1

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

brad requested this follow.
Attachment #604404 - Flags: review?(doug.turner) → review?(blassey.bugs)
Comment on attachment 604404 [details] [diff] [review]
Patch v1

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

code looks good, but I'd like to see a new patch with the refactor I describe below.

::: mobile/android/base/GeckoApp.java
@@ +2485,5 @@
> +            }
> +
> +            intent = intents.get(itemId);
> +        } else {
> +            intent = intents.get(0);

my preference would be to factor this code out into a helper function, getFilePickerIntent(String mimeType)

In that helper function, 0 and 1 possible intents can be early returns.
Attachment #604404 - Flags: review?(blassey.bugs) → review-
(Assignee)

Comment 3

5 years ago
Created attachment 605714 [details] [diff] [review]
Patch v2
Attachment #604404 - Attachment is obsolete: true
Attachment #605714 - Flags: review?(blassey.bugs)
Comment on attachment 605714 [details] [diff] [review]
Patch v2

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

::: js/src/config/check-sync-dirs.py
@@ +1,1 @@
> +#!/usr/bin/env python2

I have no idea if there is some js style guide against this. Probably easiest just to drop this change.

::: mobile/android/base/GeckoApp.java
@@ +2459,5 @@
>          ArrayList<Intent> intents = new ArrayList<Intent>();
>          PromptService.PromptListItem[] items = getItemsAndIntentsForFilePicker(aMimeType, intents);
>  
> +        if (intents.size() == 0) {
> +            Log.e(LOGTAG, "no activities for the file picker!");

this isn't really an error. Log.i would be better

@@ +2465,5 @@
> +        }
> +
> +        if (intents.size() == 1) {
> +            return intents.get(0);
> +        }

nit, no curly braces for a one line if statement
Attachment #605714 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 5

5 years ago
(In reply to Brad Lassey [:blassey] from comment #4)
> ::: js/src/config/check-sync-dirs.py
> @@ +1,1 @@
> > +#!/usr/bin/env python2
> 
> I have no idea if there is some js style guide against this. Probably
> easiest just to drop this change.

That thing had nothing to do in that patch actually. Sorry and thank you for noticing ;)
https://hg.mozilla.org/mozilla-central/rev/7e8278f5e814
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
I backed this out along with all the other patches in the initial push. Something in the push regressed Ts on Android by 20% and I don't know which patch is to blame:

https://hg.mozilla.org/mozilla-central/rev/e94edfdb1f5b

Regression Ts increase 20.6% on Android 2.2 Mozilla-Inbound
--------------------------------------------------------------
    Previous: avg 2653.656 stddev 87.856 of 30 runs up to revision 1239bd0779a6
    New     : avg 3201.178 stddev 110.202 of 5 runs since revision 0d61a0d8dba4
    Change  : +547.522 (20.6% / z=6.232)
    Graph   : http://mzl.la/zD3EWy
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/c47e0e62d6d0
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/ec0e723221ae
status-firefox13: --- → fixed
status-firefox14: --- → fixed
You need to log in before you can comment on or make changes to this bug.