1100754 - [FFOS2.0][Woodduck][Browser]Top sites display incompletely on landscape mode

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect, P2)

Description

[sync-1]

Created an attachment (id=1022208)
 image
 
 DEFECT DESCRIPTION:
 Top sites display incompletely on landscape mode
 
  REPRODUCING PROCEDURES:
 1. Into Browser and open some website
 2. Turn the device to landscape mode
 3. Top site display incompletely   ===》KO
 
  EXPECTED BEHAVIOUR:
 Can slide the page up and down to display incompletely 
 
 0752-2611942(61942)
 
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Attachment: image

Comment 2

[Josh Cheng [:josh]]

Hi Norry,
qawanted for Woodduck 2.0M and Flame 2.0/2.1/2.2. Thanks!

Comment 3

[Josh Cheng [:josh]]

Hi Gary,
Could you please help to check the problem? Thanks!

Comment 4

[Norry.L.F(Leave from Mozilla)]

Attachment: video of Flame 2.1

This issue has been successfully verified on Flame 2.1
See attachment: Flame 2.1.MP4
Reproducing rate: 0/5
Flame 2.1 versions:
Gaia-Rev        1b231b87aad384842dfc79614b2a9ca68a4b4ff3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/95fbd7635152
Build-ID        20141118001204
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141118.035447
FW-Date         Tue Nov 18 03:54:58 EST 2014
Bootloader      L1TC00011880

Comment 5

[Norry.L.F(Leave from Mozilla)]

Attachment: video of Flame 2.2

We have found, woodduck M2.0 and Flame 2.0 can't change device to landscape mode.
This issue has been successfully verified on Flame 2.2
See attachment: verify_video.mp4
Reproducing rate: 0/5
Flame 2.2 new build:
Gaia-Rev        4aee256937afe9db2520752650685ba61ce6097d
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/084441e904d1
Build-ID        20141118144012
Version         36.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141118.181013
FW-Date         Tue Nov 18 18:10:22 EST 2014
Bootloader      L1TC00011880

Comment 6

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi, I found gaia can not get orientation event, can you help to take a look?

Comment 7

[sync-1]

(In reply to comment #7)
 > Comment from Mozilla:Hi, I found gaia can not get orientation event, can you
 > help to take a look?
 > 
 
 Hi,
 
   So sorry!
 
   I don't know the gaia code. I am new in the FireFox field. 
 
   Sorry, I can't help you.
 
 Thanks!
 
 "^_^"

Comment 8

[Peipei Cheng (needinfo if you need my action)]

Hi reporter, could you please help check why gaia cannot get orientation event? we need help from your side.

Comment 9

[Josh Cheng [:josh]]

This caused by MTK which Woodduck cannot change to landscape mode now.

Comment 10

[Jin ZHANG]

(In reply to Josh Cheng [:josh] OOO from 1/5-1/10 from comment #9)
> This caused by MTK which Woodduck cannot change to landscape mode now.
> 
> *** This bug has been marked as a duplicate of bug 1103917 ***

Woodduck should be able to change to landscape mode. I dont know why but it's not MTK's issue as Flame 2.0 also fail to go landscape mode as per comment 5. You can fix this PR on Flame 2.2 first.

Hi peipei,
   would you please add me to PR1103917? cant visit it now.

Comment 11

[Peipei Cheng (needinfo if you need my action)]

Josh, this bug is different from the change orientation bug. The actual bug should be "User cannot swipe to view all topsites after they change orientation to landscape mode.

Steps:
1. Into Browser and open some website
2. Turn the device to landscape mode
   -> User cannot swipe to view all topsites after they change orientation to landscape mode.

Gary, could you please take a look?

Comment 12

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Yifan,
   Could you help this?
   Thanks.

Comment 13

[yifan [:yifan][:yliao]]

Yes will work on this later.

Comment 14

[yifan [:yifan][:yliao]]

Attachment: pull request for v2.0

Changed CSS overflow: hidden to scroll to enable scrolling.

Comment 15

[yifan [:yifan][:yliao]]

Attachment: Screenshot of scrollable top sites

Here's the screenshot after applying the patch. Thanks!

Comment 16

[yifan [:yifan][:yliao]]

Comment on attachment 8546498 [details]
Screenshot of scrollable top sites

The screenshot was taken after scrolling the top sites to the bottom.

Comment 17

[Josh Cheng [:josh]]

Hi Reporter,
Can you try patch per comment 14.

Comment 18

[Josh Cheng [:josh]]

Hi Reporter,
Can you try patch per comment 14?

Comment 19

[sync-1]

(In reply to comment #21)
 > Comment from Mozilla:Hi Reporter,
 > Can you try patch per comment 14.
 > 
 
 patch? sorry, I can't see any patch in comment 14.

Comment 20

[Ben Francis [:benfrancis]]

Comment on attachment 8546496 [details] [review]
pull request for v2.0

Thanks

Comment 21

[Stephany Wilkes]

Comment on attachment 8546498 [details]
Screenshot of scrollable top sites

Flagging Francis on ui-review here, and ni? Jacqueline as back-up while Rob and I are out and Francis is traveling.

Comment 22

[Josh Cheng [:josh]]

(In reply to sync-1 from comment #19)
> (In reply to comment #21)
>  > Comment from Mozilla:Hi Reporter,
>  > Can you try patch per comment 14.
>  > 
>  
>  patch? sorry, I can't see any patch in comment 14.

It is here: https://bugzilla.mozilla.org/attachment.cgi?id=8546496

Comment 23

[Francis Djabri [:djabber]]

Hi Eric, could you review this against your visual spec please?
Thanks, Francis

Comment 24

[Eric Pang [:epang] UX]

Attachment: Browser_Start_Page_Specs.jpg

Hi, can we update the screens to show 3 across?  This will help utilize the available space and minimize scrolling. I've attached a spec I created for the 2.1 browser start page.  I realize this spec is not 2.0, but the same concept can be followed for the browser screens. Please let me know if you have any questions.  Also, feel free to flag me for ui-review when ready, thanks!

Comment 25

[yifan [:yifan][:yliao]]

Hi Eric, thank you! Should we also remove the 'Web Browser' header? If we could remove that and minimize the vertical margin between thumbnails, users wouldn't have to scroll down for the top sites at bottom row.

Comment 26

[Eric Pang [:epang] UX]

(In reply to yifan [:yifan][:yliao] from comment #25)
> Hi Eric, thank you! Should we also remove the 'Web Browser' header? If we
> could remove that and minimize the vertical margin between thumbnails, users
> wouldn't have to scroll down for the top sites at bottom row.

Yes, that makes sense to me, the user will already know they are in the browser when opening the app.  Can we also center 'Top Sites'?  Thanks!

Comment 27

[Jacqueline Savory [:jsavory] UX]

Removing flag as the changes Eric has suggested look great to me.

Comment 28

[yifan [:yifan][:yliao]]

Attachment: updated portrait screenshot on flame

Thank you Eric! Here's the updated topsites list in portrait mode.

Comment 29

[yifan [:yifan][:yliao]]

Attachment: updated landscape screenshot on flame

And here's the screenshot for landscape mode.

Comment 30

[yifan [:yifan][:yliao]]

Comment on attachment 8546496 [details] [review]
pull request for v2.0

Hi Ben, I made some modifications following the feedback from UX. Thank you for reviewing the code change!

* Enable scrolling on the start screen.
* Remove web browser text on the start screen.
* Change the NO. of displayed top sites from 4 to 6.
* Change CSS float layout to flex layout.

Comment 31

[Eric Pang [:epang] UX]

(In reply to yifan [:yifan][:yliao] from comment #30)
> Comment on attachment 8546496 [details] [review]
> pull request for v2.0
> 
> Hi Ben, I made some modifications following the feedback from UX. Thank you
> for reviewing the code change!
> 
> * Enable scrolling on the start screen.
> * Remove web browser text on the start screen.
> * Change the NO. of displayed top sites from 4 to 6.
> * Change CSS float layout to flex layout.

Hi Yifan, this is looking much better!  But can you check the font?  It doesn't seem to be using Fira Sans.  Can you update and re post screens?  Sorry for the trouble, thanks!

Comment 32

[yifan [:yifan][:yliao]]

Attachment: fira font portrait screenshot

Thank you for the review! Updated the screenshots with fira font.

Comment 33

[yifan [:yifan][:yliao]]

Attachment: fira font landscape screenshot

Comment 34

[Ben Francis [:benfrancis]]

Comment on attachment 8546496 [details] [review]
pull request for v2.0

I'm sorry but we really shouldn't be making changes like this to 2.0 at this stage. This patch doesn't just fix the reported blocking bug, it makes changes to content and presentation which introduces risks of further regressions.

If this code still existed on master I would r+ it so it went into the next release, but it doesn't and in my view it's too late to be making patches like this directly to the 2.0 branch if we ever want it to be stable.

Comment 35

[Eric Pang [:epang] UX]

Comment on attachment 8551075 [details]
fira font portrait screenshot

This looks better now, thanks!

Comment 36

[Eric Pang [:epang] UX]

Comment on attachment 8551076 [details]
fira font landscape screenshot

Looks good, thanks!

Comment 37

[yifan [:yifan][:yliao]]

Thank you Ben and Eric! I'll revert it back to the original fix.

Comment 38

[yifan [:yifan][:yliao]]

Attachment: pull request for v2.0

Enable scrolling for the start screen. Thanks for the review!

Comment 39

[Ben Francis [:benfrancis]]

Comment on attachment 8551583 [details] [review]
pull request for v2.0

Thank you

Comment 40

[yifan [:yifan][:yliao]]

Thank you!

Merged into v2.0m https://github.com/mozilla-b2g/gaia/pull/27555

Comment 41

[Kai-Zhen Li [:seinlin][:kli]]

Per comment 40.

Comment 42

[SandKing]

Attachment: verified video:verify.mp4

This issue(Enable scrolling for the start screen) has been verified successfully on Woodduck 2.0, but other modifications on comment 30 wasn't changed
Verify video:"verify.mp4"

Wooddduck 2.0M:
Build ID               20150227050313
Gaia Revision          a1b5959728c8bc2a82354e197bb161922d419866
Gaia Date              2015-02-13 09:00:02
Gecko Revision         d9b299dc1087f23c83321b4dccc92e0f52309e8e
Gecko Version          32.0
Device Name            jrdhz72_w_ff
Firmware(Release)      4.4.2
Firmware(Incremental)  1424984723
Firmware Date          Fri Feb 27 05:05:41 CST 2015