1001129 - Clicking Download icon is required in nightly, no error feedback

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Download Manager, defect)

Description

[Austin King [:ozten]]

I wanted to download and install an .apk.

In nightly, Android asks me to use "Downloader" to download it.

Several times I didn't actually click the Downloader icon and just hit "once" or "evertime".

This dismisses the dialog without downloading anything, leaving me very confused.

Expected - either pre-select the icon or give the user feedback that they need to click the icon. We shouldn't silently close the dialog.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's get the "Download" intent selected by default.

Filing a separate bug for fixing this with a banner.

Comment 2

[Wesley Johnston (:wesj)]

Err.. I did this the other way around. I'm not a big fan of the "banner" idea. I'd like to try and reduce the number of UI's we have for what is essentially "Intent" selection rather than grow it.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

This forces us to have a default selection in GridViews. There was already code here for this (sorta)! What we refer to as selected is actually checked to Android, so even if we did select something, it wasn't working.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8418543 [details] [diff] [review]
Patch

We should probably try to uplift this

Comment 5

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/fx-team/rev/ab1394be8e93

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backed out for robocop crashes.
https://hg.mozilla.org/integration/fx-team/rev/32acf0cd2a16

https://tbpl.mozilla.org/php/getParsedLog.php?id=39502494&tree=Fx-Team

Comment 7

[Aaron Train [:aaronmt]]

Re-land?

Comment 8

[Wesley Johnston (:wesj)]

The android docs lie. This method is only implemented on ics plus. I've been trying to reflect out an alternative for older version (setSelectedPositionInt()). Almost there, but if it fails, we'll still have this item "selected". It just won't appear that way visually.

Comment 9

[Wesley Johnston (:wesj)]

Attachment: Patch v2

This just wraps this call in a version check.

I tried reflecting out setSelectedPositionInt from AdapterView:

http://androidxref.com/2.3.6/xref/frameworks/base/core/java/android/widget/AdapterView.java#1086

but we don't actually style the "selected" item like I'd hoped. A quick attempt to do that failed, and I decided it wasn't worth putting a lot of time into. Happy to try more if I'm wrong.

For any interested reader, you might think that setSelection would do the same thing for you:

http://developer.android.com/reference/android/widget/AdapterView.html#setSelection%28int%29

It turns out, setSelection is does nothing on Android for devices that are "inTouchMode" (i.e. devices with touch screens). I assume that's some UX decision that you shouldn't be able to programmatically set the selection if  user could touch a widget, but I haven't found any good explanations from any Android engineers. Just lots of code paths that dead end in inTouchMode checks.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8426431 [details] [diff] [review]
Patch v2

Will this patch set all intent chooser dialogs to select the first item? I see mSelected inited to 0. Shouldn't we leave mSelected: -1 ?

We only want the HelperAppDialog to do this, and you are setting |selected: true| there.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8426431 [details] [diff] [review]
Patch v2

Wes and I chatted on IRC. On ICS+, the item we selected will actually appear selected.

I do want the | mSelected: 0 | part reverted for now. I like being explicit about selecting by default. We can change our minds later too.

Comment 12

[Wesley Johnston (:wesj)]

Also made sure we don't try to select if mSelection == -1
https://hg.mozilla.org/integration/fx-team/rev/fb993f33e948

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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