833003 - Viewport doesn't work intelligently if the page is set viewport=device-width & has the content which is wider than device width

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: testcase

[Envirnment]
* http://hg.mozilla.org/mozilla-central/rev/91b9995f9ac7
* Xperia arc (ROM: ICS Sony build)

[Step to reproduce]
At Firefox for Android, Open the attached testcase, or http://blog.livedoor.jp/geek/lite/archives/51362883.html .
These pages has following  characteristices
* viewport config: width=device-width
* viewport config: user-scalable=no
* Has the content which is wider than the device width.

[Result]
Viewport doesn't work intelligently. These pages has the config "user-scalable=no". So user cannot browse contents comfortabily. This is very annnoying.
(Please see screenshot which I'll attach in next comment.)

[More info]
* Stock browser (4.0.4), Chrome for Android (beta 25), and Opera Mobile (12.1) work intelligently. They follow "user-scalable=no", but they provide the page as scrollable.
* At Firefox for Android, if user sets the config "browser.ui.zoom.force-user-scalable=true", Firefox provides similar rendering to other browsers. And user can scroll the page content. So this bug is relaxation.

[Expection]
Firefox Android handles the content which has these case intelligently.

Comment 1

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: screenshot

Comment 2

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

I seems this bug relates to http://hg.mozilla.org/mozilla-central/file/5835bc763be7/mobile/android/base/gfx/JavaPanZoomController.java#l811

Comment 3

[Kartikaya Gupta (email:[e-mail])]

That is one of the many lines of code involved, yes. At this point I'm not really planning to put in more fixes to JavaPanZoomController unless it's critical because we are planning to get rid of the JavaPanZoomController class and use the AsyncPanZoomController class from the B2G code instead. Bug 745136 is tracking this work overall.

Comment 4

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> That is one of the many lines of code involved, yes. At this point I'm not
> really planning to put in more fixes to JavaPanZoomController unless it's
> critical because we are planning to get rid of the JavaPanZoomController
> class and use the AsyncPanZoomController class from the B2G code instead.
> Bug 745136 is tracking this work overall.

I found this bug's cause just a little while ago.
This bug is caused with that viewport metadata.defaultZoom doesn't have any value. But I think metadata.scaleRatio can be used as its value when defaultZoom has no value. (And I confirmed it works well if we change it so.)
I'm making the patch & testing it now.

Kartikaya, Could you review the patch after finished test?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Yes, if you have a patch I can review it.

Comment 6

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #5)
> Yes, if you have a patch I can review it.

Thank you!

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 712513 [details] [diff] [review]
patch

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

This looks good, thanks!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/113113124e49

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/113113124e49