XUL files opened via Browser.addTab do not display

VERIFIED FIXED

Status

--
major
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: cfinke, Assigned: vingtetun)

Tracking

Trunk
x86
Mac OS X
Bug Flags:
in-litmus -

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5
Build Identifier: 

I've found that XUL files opened via Browser.addTab do not display, although opening them by manually typing the URL in the location bar displays them properly.

Reproducible: Always

Steps to Reproduce:
1. Add a button via an extension (or whatever method).
2. Have it run "Browser.addTab('about:config');" oncommand.
3. Click it.
Actual Results:  
A blank tab opens, albeit with the correct title.

Expected Results:  
The XUL file should open in the tab.
(Reporter)

Comment 1

9 years ago
Created attachment 412122 [details]
An extension that adds a button to the right toolbar that exhibits the bug behavior.
Not sure why viewportRect.width && viewportRect.height stay both at 1 pixel.

Others people has confirmed it too.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Ok. we are broken since changeset 1060:3d8f7d7b2fc7
Got this bug too.

Comment 5

9 years ago
This problem seems not to be specific for XUL files. It also occurs, if you open a new tab using an URI pointing to a simple HTML file.
I'm getting reports from add-on developers that this bug is a serious problem
tracking-fennec: --- → ?

Updated

9 years ago
tracking-fennec: ? → 1.0+
Yep it's a blocking bug for my SamplerBox add-on :/
(Reporter)

Comment 8

9 years ago
(In reply to comment #7)
> Yep it's a blocking bug for my SamplerBox add-on :/

Ditto for ScribeFire.
1 pixel viewport makes me think the browser is never getting the proper scroll size events (viewport starts out as 0,0,1,1).  The background patch listens for all size change events, but Vivian and I have confirmed this is still occurring.
Apparently this come because MozScrolledAreaChanged is not fired for Xul documents.

froystig is working on that (don't have a bug number yet)

Comment 11

9 years ago
Here's a workaround : 

function loadXUL(uri) {
  Browser.addTab('about:blank', true);
  window.setTimeout(function() {
         Browser.selectedBrowser.loadURI(uri, null, null)}, 0);
}
What if we use BrowserUI.newTab? Does that work better?

(In reply to comment #11)
> Here's a workaround : 
> 
> function loadXUL(uri) {
>   Browser.addTab('about:blank', true);
>   window.setTimeout(function() {
>          Browser.selectedBrowser.loadURI(uri, null, null)}, 0);
> }

This implies a timing issue of some kind.
(Reporter)

Comment 13

9 years ago
(In reply to comment #12)
> What if we use BrowserUI.newTab? Does that work better?

No; BrowserUI.newTab has the same results as Browser.addTab for the test case.
(In reply to comment #11)
> Here's a workaround : 
> 
> function loadXUL(uri) {
>   Browser.addTab('about:blank', true);
>   window.setTimeout(function() {
>          Browser.selectedBrowser.loadURI(uri, null, null)}, 0);
> }

This works because about:blank receives a scrollarea change event.  about:config or apparently any XUL files do not get the event.
Created attachment 415867 [details] [diff] [review]
Patch

Patch for handling xul document by creating a fake MozScrolledAreaChanged event and by using our old getBrowserDimensions method.

Why do we need this?

Because the bounds of a xul doc didn't change during reflow and so we don't fire the MozScrolledAreaChanged event (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#896).

Also this code (for fire the event) belongs to nsHTMLScrollFrame not a nsXULScrollFrame for xul documents, I'm not sure why but i've fall in the trap and lose some times on that. (if someone know the answer?)
Created attachment 415868 [details] [diff] [review]
Patch

The same without my changes to browser.xul
Attachment #415868 - Flags: review?(webapps)
Created attachment 415884 [details] [diff] [review]
Patch v0.2
Attachment #415868 - Attachment is obsolete: true
Attachment #415884 - Flags: review?(webapps)
Attachment #415868 - Flags: review?(webapps)
Attachment #415868 - Flags: review?(mark.finkle)
Comment on attachment 415884 [details] [diff] [review]
Patch v0.2

>   createBrowserViewportState: function createBrowserViewportState() {
>-    return new BrowserView.BrowserViewportState(new Rect(0, 0, 1, 1), 0, 0, 1);
>+    return new BrowserView.BrowserViewportState(new Rect(0, 0, kDefaultBrowserWidth, window.innerHeight), 0, 0, 1);
>   },

Can't just use window.innerHeight, calculate the height like resize handler does.  Also, innerWidth and innerHeight are 0 when Fennec loads.  There's some assumptions in TileManager about the width and height being at least 1, so check for this.

>   getViewportStateFromBrowser: function getViewportStateFromBrowser(browser) {
>     return browser.__BrowserView__vps;
>   },

Forgot about this function.  Its purpose is long gone, let's get rid of it sometime.

>+
>+      if (browser.contentDocument instanceof XULDocument && Browser.selectedBrowser == browser)
>+        Browser._browserView.zoomToPage();

Is this to fix the zoom problem for XUL pages?  It didn't do the job for me.  In fact, I can still scroll around as if the page has the dimensions of the old page.

It looks like MozScrollAreaChange is not fired even when moving from an HTML page to a XUL. :( If this is true, maybe your secondmost recent patch is the only way to go.
Attachment #415884 - Flags: review?(webapps) → review-
> >+
> >+      if (browser.contentDocument instanceof XULDocument && Browser.selectedBrowser == browser)
> >+        Browser._browserView.zoomToPage();
> 
> Is this to fix the zoom problem for XUL pages?  It didn't do the job for me. 
> In fact, I can still scroll around as if the page has the dimensions of the old
> page.

Weird, i should have done something wrong between my test and the patch :/

> 
> It looks like MozScrollAreaChange is not fired even when moving from an HTML
> page to a XUL. :( If this is true, maybe your secondmost recent patch is the
> only way to go.

This is true. MozScrolledAreaChanged is never fired for xul docs (or at least for all the cases i've test.). but my second patch was supposed to made the stuff works - I'll do another iteration tomorrow to see what's wrong
Comment on attachment 415868 [details] [diff] [review]
Patch

I'm not sure how the previous patch i have made could work since i didn't updating the viewport, i'm not sure why it has work during my test, i've probably do some weird stuff...

So, yeah, this patch is probably the way to go. It has also an other benefits which is addons developers can listen for MozScrolledAreadChanged vent and react to this - so finally doing that is probably not so bad.
Attachment #415868 - Attachment is obsolete: false
Attachment #415868 - Flags: review?(webapps)
Attachment #415868 - Flags: review?(webapps)
Comment on attachment 415868 [details] [diff] [review]
Patch

Considering the above, looks good to me.
Attachment #415868 - Flags: review+
Pushed http://hg.mozilla.org/mobile-browser/rev/03cfe0e88a8d
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
great :)
verified FIXED on builds:
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b5pre) Gecko/20091214 Firefox/3.6b5pre Fennec/1.0b6pre

and

Mozilla/5.0 (X11; U; Linux armv6l; Nokia N8xx; en-US; rv:1.9.3a1pre) Gecko/20091214 Firefox/3.7a1pre Fennec/1.0b5
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Flags: in-litmus? → in-litmus-
You need to log in before you can comment on or make changes to this bug.