Closed Bug 613480 Opened 9 years ago Closed 9 years ago

Check for a NULL docShell in browser constructor

Categories

(Toolkit :: XUL Widgets, defect)

x86_64
Windows Server 2003
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
In Firefox Mobile, where multi-process web content is used, the browser.docShell is NULL. The browser XBL constructor uses the docShell without checking it first and causes errors when run in Firefox Mobile.

This patch just adds a simple check to the constructor.
Attachment #491826 - Flags: review?(gavin.sharp)
In addition to errors reported by QA and users, this bug will keep the Mobile browser-chrome tests from ever being green, since the word "Error" is found in the browser-chrome output.
When is the docShell initialized?
(In reply to comment #2)
> When is the docShell initialized?

For remote browsers? It's never available in the chrome (parent) process. Only local browsers have a docShell available to the browser binding.

For remote browsers the docShell is available to the content process scripts.
Does my answer make sense? Let me know if you need any more explanation.
A browser binding whose .docShell is always null wouldn't be very useful, so I assumed that patch was working around the fact that the docShell is null when the constructor runs, but non-null at some later point. If you're saying instead that this.docShell is always null, then I don't understand how you wouldn't be running into a bunch of other problems...
OS: Linux → Windows Server 2003
So as explained on IRC you don't run into other problems because you're using bindings that override the base browser binding and re-implement most of its functionality. So it's just the constructor (which you can't prevent from running) that poses a problem.
Attachment #491826 - Flags: review?(gavin.sharp) → review+
Comment on attachment 491826 [details] [diff] [review]
patch

Removes extraneous errors on console, which get reported as bugs. Also needed for Fennec to pass browser-chrome tests
Attachment #491826 - Flags: approval2.0?
Attachment #491826 - Flags: approval2.0? → approval2.0+
pushed:
http://hg.mozilla.org/mozilla-central/rev/b5610f4fa3a8
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
bugspam
Assignee: nobody → mark.finkle
You need to log in before you can comment on or make changes to this bug.