The default bug view has changed. See this FAQ.

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.