946172 - [Dialer] Call screen is broken with APZ turned on

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

The call screen seems to be only rendered half part with APZ enabled. Sounds pretty bad.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Steps to reproduce:
 - make a phone call or receive one.
 - wait for the call screen to appears
 - hit the home button

Expected results:
 - The call screen is minimized and a small green view is visible

Actual result:
 - The call screen is minimized but it does not have changed in the background


 - Hit the home button again

Expected result:
 - The call screen is reopened and take the full height (minusing the statusbar)

Actual result:
 - Only half of the call screen renders

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

It seems like the first issue with the call screen comes from the fact that we resize the container window in this case. What happens without APZC is that the viewport is basically resize to the width/height of this window. It seems like with APZC the height is not changed and so that's why the css media queries does not apply.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Seems like the artificial limit of a min-height of 223 pixels imposed by http://hg.mozilla.org/mozilla-central/annotate/7476bb2b8e9c/content/base/public/nsViewportInfo.h#l17 does not help where we want 40.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug946172.minheight.viewport.patch

The first part of the issue is solved with this.

For the second part it seems like the call screen redraw when you touch the screen. I feel like I have seen a similar issue today with the call screen where you have to touch the screen to have it repaint. I need to find the bug number though and this is probably an orthogonal issue.

Matt, I'm asking you the review hoping that you know where this |223| comes from. In the worst case we can have this value defined on a per platform if there is really something that rely on this stuff.

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8342400 [details] [diff] [review]
bug946172.minheight.viewport.patch

Review of attachment 8342400 [details] [diff] [review]:
-----------------------------------------------------------------

223 originally came from bug 561870, which mentions that it was copied from this Safari documentation:
https://developer.apple.com/library/safari/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/MetaTags.html

I don't know of anything that depends on this exact number.  It should be fine to change it.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Matt Brubeck (:mbrubeck) from comment #5)
> Comment on attachment 8342400 [details] [diff] [review]
> bug946172.minheight.viewport.patch
> 
> Review of attachment 8342400 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> 223 originally came from bug 561870, which mentions that it was copied from
> this Safari documentation:
> https://developer.apple.com/library/safari/documentation/AppleApplications/
> Reference/SafariHTMLRef/Articles/MetaTags.html
> 
> I don't know of anything that depends on this exact number.  It should be
> fine to change it.

Thanks for the historical commit, I knew it was you!

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://tbpl.mozilla.org/?tree=Try&rev=05309814d51b

Seems like a test expect 223. Rebuilding.

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #7)
> https://tbpl.mozilla.org/?tree=Try&rev=05309814d51b
> 
> Seems like a test expect 223. Rebuilding.

Seems green this time. https://tbpl.mozilla.org/?tree=Try&rev=8ae2ce02ae49

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1bc26beda606

Comment 10

[Ryan VanderMeulen [:RyanVM]]

(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #9)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/1bc26beda606

Can you please push B2G work to b2g-inbound? It's where it's supposed to land and landing on m-i increases the risk of merge conflicts.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1bc26beda606