1383898 - Send to Device in Page Action Menu lacks tablet icon

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P2)

Description

[Ryan Feeley [:rfeeley]]

Attachment: ipad.png

STEPS TO REPRODUCE
1. Enable Sync in dektop
2. Enable Sync on a tablet device
3. Open Send to device in Page Action Menu

EXPECTED RESULTS
- Tablet icon is shown

ACTUAL RESULTS
- Smartphone icon is shown

Comment 1

[Edouard Oger [:eoger]]

>[17:39:12]  <eoger>	iOS does write formfactor = "tablet" at least
>[17:39:16]  <eoger>	so it should be do-able
>[17:39:20]  <eoger>	do we have the svg for that?

I'm also concerned by the fact that the Android tablet detection [0] might not be really "rock solid" with phone screens getting bigger and bigger every year. What do you think Richard? We could also activate that icon only with iOS devices, who have way better formfactor detection capabilities.

[0] http://searchfox.org/mozilla-central/source/mobile/android/thirdparty/com/adjust/sdk/DeviceInfo.java#122

Comment 2

[Richard Newman [:rnewman]]

(In reply to Edouard Oger [:eoger] from comment #1)

> I'm also concerned by the fact that the Android tablet detection [0]

That's actually Adjust's code; it's nothing to do with Sync.

Sync uses this:

https://dxr.mozilla.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareUtils.java#66

which is this:

        if (screenLayoutSize == Configuration.SCREENLAYOUT_SIZE_XLARGE) {
            sIsLargeTablet = true;
        } else if (screenLayoutSize == Configuration.SCREENLAYOUT_SIZE_LARGE) {
            sIsSmallTablet = true;
        }

like this:

https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/src/main/java/org/mozilla/gecko/sync/SharedPreferencesClientsDataDelegate.java#106

That code is used throughout Fennec.


> not be really "rock solid" with phone screens getting bigger and bigger
> every year. What do you think Richard?

There is a concern here, yes.

"Large" ("smalltablet") roughly corresponds to 4-7" devices. This is meant to catch the Galaxy Note and so on.
"XLarge" ("largetablet") roughly corresponds to 7-10" devices.

https://developer.android.com/guide/practices/screens_support.html

The important thing is that this agrees with the devices on which we show the Firefox tablet UI.

A good way to test this concern: what value do we use for a 5" phone? Does it use the Firefox phone UI? (It should.)

Comment 3

[Richard Newman [:rnewman]]

Of course, whether the formfactor field is set correctly is really a totally separate bug (which I encourage you to file if you have reasonable doubts!); IMO if you have a client record that claims to be from a small or large tablet, use the tablet icon.

Comment 4

[Edouard Oger [:eoger]]

> That's actually Adjust's code; it's nothing to do with Sync.
My bad, I didn't notice the path :)

> what value do we use for a 5" phone?
I use a Nexus 5X which has a screen size of 5.2" and getting the phone UI.

Thank you for your clarifications Richard.
Let's move on with this.

Comment 5

[Bram Pitoyo [:bram]]

Attachment: device-tablet-16.svg

Here’s the tablet icon in SVG format.

Comment 6

[Edouard Oger [:eoger]]

Attachment: Bug 1383898 - Add tablet icon to Send Page to Device panel.

Review commit: https://reviewboard.mozilla.org/r/174464/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/174464/

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8902797 [details]
Bug 1383898 - Add tablet icon to Send Page to Device panel.

https://reviewboard.mozilla.org/r/174464/#review180228

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b28fb2072399
Add tablet icon to Send Page to Device panel. r=markh

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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