903913 - [Flatfish]Need script to detect device type for reuse in shared folder

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[(Inactive after June) George Duan [:gduan] [:喬智]]

Need a script that can detect device type for other apps to use.

Comment 1

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

This patch is to provide a simple script that can tell apps what kinda device we're using.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 788816 [details]
PR to master

r+, Let's not land this patch to master until we have it settled on the product side.

Comment 3

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Tim,

I update my patch for below reasons.
1. if screen is 1280x800 and app is opened in portrait mode, then my original isLarge may return us wrong value.
2. add isPortrait and isLandscape api
3. add orientationObserver
4. function name and file name may cause some confusion, so I update the name as screen_layout

Could you kindly check again? thanks.

Comment 4

[Marco Chen [:mchen]]

Hi all,

Bug 788975 tried to use accelerometer sensor to detect screen rotation because 
  1. some low end device didn't have orientation sensor.
  2. Orientation sensor consumed more power then accelerometer sensor.

So if you use deviceorientation to calculate the screen orientation then it will not work on some low end device. (but auto rotation is workable.)

Comment 5

[Marco Chen [:mchen]]

HI George & Tim,

Please take comment 4 into consideration. Thanks.

Comment 6

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Macro,

Thanks for your comment 4. I will listen mozorientationchange instead of deviceorientation.

Comment 7

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 788816 [details]
PR to master

Hi Tim,

I think I should remove isPortrait, isLandscape, and orientationObserver, since mozorientationchange event already do a great job on it and also window.screen.orientation which tell us portrait/landscape with primary/secondary.

However, I still made some change on this patch. Considering app is opened in portrait mode under 1280x800 screen, the original api will not tell correct result, so I choose maximum value of height and width to identify device type.

Could you kindly review again? Thanks.

Comment 8

[[e-mail]]

In last week offline discussion, George and I decide to not use maxWidth (get max value from width/height) here because current break point (768px from bootstrap) is sufficient to distinguish for most bigger device(ex 1080x800 or 1200x800). Either 1200, 1080, or 800 exceed 768px, thus generally tablet will always be detected as 'medium' device.

Comment 9

[[e-mail]]

Comment on attachment 788816 [details]
PR to master

looks good to me, except need to fix a typo shown in github comment

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Discussed offline. Revised patch to come later today.

Comment 11

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 788816 [details]
PR to master

Hi Tim,

This patch has added getCurrentLayout method and rename addQueryWatcher/removeQueryWatcher as watch/unwatch and also update some comment. Please kindly review it again. Thanks.

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 788816 [details]
PR to master

Thank you! Let's start landing large screen patches :)

Comment 13

[(Inactive after June) George Duan [:gduan] [:喬智]]

Thanks Tim.

Merge into master
https://github.com/mozilla-b2g/gaia/commit/7cfd4679d681a2a231a01ebe7c867b3b562c095c

Comment 14

[Ed Morley [:emorley]]

Backed out for breaking the build, eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=27111057&tree=B2g-Inbound
(and yeah logs are pretty useless lol)

https://github.com/mozilla-b2g/gaia/commit/e343634ff8077cce1343294dc5d6c5fe586c5668

Please test locally before pushing! :-)

Comment 15

[Ed Morley [:emorley]]

This might have been false blame by the gaia commit robot cooincidentally pushing at around the same time (turns out we don't use gaia.json properly all the time), and the bustage here might have been caused by bug 904068.

Comment 16

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Ed,

I don't think it's my patch's problem. Should I merge into master again?

Comment 17

[Ed Morley [:emorley]]

Yup :-)

Comment 18

[(Inactive after June) George Duan [:gduan] [:喬智]]

Merge into master,
https://github.com/mozilla-b2g/gaia/commit/c8ee5fc7d7247093113eb8f89465e1121c866f3c