547927 - Page layout broken if some wider page has been visited before

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chwiej Pawel]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.1.8) Gecko/20100202 Firefox/3.5.8
Build Identifier: Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2.2pre)  Gecko20100222 Namoroka/3.6.2pre Fennec/1.1a2pre

Maemo 5
fennec_1.1~a2~pre_armel.deb 20100222


Reproducible: Always

Steps to Reproduce:
1. Go to youtube.com
2. Once the page is loaded, go to mns.com

Actual Results:  
Mns page occupies only part of the screen (check screenshot).


Expected Results:  
Mns Page should fit the view, no white, empty space on the right or at the bottom. You can see the proper view if you skip the first step.

Comment 1

[Chwiej Pawel]

Attachment: screenshot

Comment 2

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

I'm not sure of what's happening here but it sounds like MozScrolledAreaChanged is not called.

Comment 3

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

Attachment: Patch v0.1

This patch correct the bug for this website and also a very similar on http://www.yourhtmlsource.com/examples/frameset1.html.

One difference thought, while the first return an empty metaData.reason, the second return a metaData.reason equal to "viewport", that's why I've move the check to HTMLFrameSetElement at the top. The other possible way is to hack directly into contentIsHandheld to not return a "viewport" reason in this case.

Comment 4

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

Just for comprehension, this happen because MozScrolledAreachanged is not applied for this type of document (HTMLFrameSetElement), and because of the check at the start of the handler we're doing nothing for sub-documents: http://mxr.mozilla.org/mobile-browser/source/chrome/content/BrowserView.js#537

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 428457 [details] [diff] [review]
Patch v0.1

>+    if (browser.contentDocument instanceof XULDocument ||
>+        browser.contentDocument.body instanceof HTMLFrameSetElement) {
>+       // XXX XUL documents do not receive scroll change event
>+       let [w, h] = BrowserView.Util.getBrowserDimensions(browser);
>+       let event = document.createEvent("Event");
>+       event.initEvent("MozScrolledAreaChanged", true, false);
>+       event.x = 0;
>+       event.y = 0;
>+       event.width = w;
>+       event.height = h;
>+       browser.dispatchEvent(event);

Maybe this should be it's own if block, separate from the metaData tests

>+    } else if (metaData.reason == "handheld" || metaData.reason == "doctype") {

And this would be the start of the "handheld" if block


Do we need these two very different types of checks to be combined into one if block?

Comment 6

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

Attachment: Patch v0.2

So, I guess you would prefer something like that.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 428550 [details] [diff] [review]
Patch v0.2

Indeed

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/3953d0381f1a

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED On builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.3pre) Gecko/20100402 Namoroka/3.6.3pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.3a4pre) Gecko/20100402 Namoroka/3.7a4pre Fennec/1.1a2pre