1203628 - "Send to other devices" from Share overlay in other apps is styled incorrectly

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Overlays, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: Screenshot_2015-09-10-10-03-22.png

STR:
 - "Add to Firefox"
 - Press "Send to other devices"
 - See ICS styled Modal

Expected:
 - Same style for devices list as "Send to other devices" while in Firefox

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_shareoverlay_device.png

Comment 2

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Worth noting that with 2 (or is it 3?) other synced devices, as most users have, the device list is shown inline so this won't affect most users.

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

I can't remember either, did we decide on 2 or 3 devices because of some data or was it more arbitrary than that? 

Also a modal might not be so bad for a long list of devices... if it at least matched our theme (L). :)

Comment 4

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

(In reply to Anthony Lam (:antlam) from comment #3)
> I can't remember either, did we decide on 2 or 3 devices because of some
> data or was it more arbitrary than that? 

It was that way when I got to it, but I'd infer it had something to do with the fact that users generally have fewer than 2 or 3 other devices synced and wanting to keep context from the previous app.

> Also a modal might not be so bad for a long list of devices... if it at
> least matched our theme (L). :)

Do we have other dialogs that match our theme? If so, which are those (besides the share overlay dialog)?

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Michael Comella (:mcomella) from comment #4)
> (In reply to Anthony Lam (:antlam) from comment #3)
> > I can't remember either, did we decide on 2 or 3 devices because of some
> > data or was it more arbitrary than that? 
> 
> It was that way when I got to it, but I'd infer it had something to do with
> the fact that users generally have fewer than 2 or 3 other devices synced
> and wanting to keep context from the previous app.

I think you might be right. Also because we only had 3 items in the list at the time. How hard would it be to just keep it the same as what we have as our "Shareplane" list UI that slides up?

> > Also a modal might not be so bad for a long list of devices... if it at
> > least matched our theme (L). :)
> 
> Do we have other dialogs that match our theme? If so, which are those
> (besides the share overlay dialog)?

Yep, all our dialogs match the L theme. Not sure why this one is not doing it. Might be worth a check. I sent you a link to another bug in about:logins that uses L theme dialogs for example.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

(In reply to Anthony Lam (:antlam) from comment #5)
> How hard would it be to just keep it the same as what we have as
> our "Shareplane" list UI that slides up?

The code would probably be gross and hacky, but I think it can be done. Annoying but not too hard.

> Yep, all our dialogs match the L theme. Not sure why this one is not doing
> it. Might be worth a check. I sent you a link to another bug in about:logins
> that uses L theme dialogs for example.

Might be that the ShareOverlay Activity inherits from a different Activity - another reason for bug 1202076!

Comment 7

[Sergej Kravcenko]

Line 127 in SendTabList.java causes the problem, "builder = new AlertDialog.Builder(context, R.style.Gecko_Dialog);". It sets some custom styling.

Anyway, about:logins uses default AlertDialog inside Prompt.java without the title, so it kinda looks the same, but in reality what you will see depends on Android version.

If you remove "Select device" title and remove that line, it will look like about:login dialog, but it still be different on various Android versions.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Attachment: Post patch

That looks right – thanks Sergej!

Comment 9

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Attachment: MozReview Request: Bug 1203628 - Don't use gecko style on share overlay dialog. r=sebastian

Bug 1203628 - Don't use gecko style on share overlay dialog. r=sebastian

Comment 10

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

(btw, feel free to ask to be assigned or just post a patch in the future, Sergej!)

Comment 11

[Richard Newman [:rnewman]]

(In reply to Michael Comella (:mcomella) from comment #2)
> Worth noting that with 2 (or is it 3?) other synced devices, as most users
> have, the device list is shown inline so this won't affect most users.

Bug 1122977.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8663212 [details]
MozReview Request: Bug 1203628 - Don't use gecko style on share overlay dialog. r=sebastian

https://reviewboard.mozilla.org/r/19751/#review17825

Comment 13

[Pulsebot]

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

Comment 14

[Carsten Book [:Tomcat]]

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

Comment 15

[u549602]

Verified as fixed on latest Nightly(44.0a1 10/01/2015) Android using Sony Xperia Z2 (Android 5.0.2)