Justin Dolske [:Dolske] Assignee
	Description • 14 years ago

Now that bug 130078 is fixed, chrome can put stuff over content. For tab-modal prompts, I need to overlay the entire <browser> with a prompt that blocks interaction with the page. This would be easiest if the browser was in a <stack>, so here we go.

This is based on the work Frank did in bug 583462. Currently running through tryserver to see if any tests fail because they make assumptions about the DOM that are being changed here.

	Justin Dolske [:Dolske] Assignee
	Comment 1 • 14 years ago

Changes to some tests and code (HUDService), which depened on the old DOM structure.

Dunno if it's worth adding an API to go from the <browser> to the <notificationbox> instead of using .parentNode.parentNode? Maybe?

	Justin Dolske [:Dolske] Assignee
	Comment 2 • 14 years ago

Oh, actually, I think maybe we should, but instead as browser.linkedTabPanel?

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 3 • 14 years ago

We should add a property to browsers that links to their tabpanels. Don't really want it to be a strong reference, though, and isn't something that we can enforce (since browsers aren't all necessarily contained within tabpanels). Perhaps a "linkedTabPanel" property that returns document.getElementById(this.getAttribute("linkedpanelid")), and have the tabbrowser code set linkedpanelid attribute on the browser when it creates it?

	Dão Gottwald [:dao]
	Comment 4 • 14 years ago

(In reply to comment #3)
> Perhaps a "linkedTabPanel" property that returns
> document.getElementById(this.getAttribute("linkedpanelid"))

Those are anonymous nodes as far as tabbrowser is concerned.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 5 • 14 years ago

Huh? The <notificationbox>s aren't anonymous nodes.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 6 • 14 years ago

To be clear, something like this is what I was suggesting.

	Dão Gottwald [:dao]
	Comment 7 • 14 years ago

The look very anonymous over here. See also bug 508819.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 8 • 14 years ago

Bah, I thought they were <content>-ed into tabbrowser (and that I was looking at browser.xul). Time to sleep, I guess.

We could still set .linkedPanelId on the browser, and have a getPanelForBrowser() helper on tabbrowser, I guess. Would be nice to nicely expose the <stack> itself somehow too, but having many "weak up references" is starting to get messy...

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 9 • 14 years ago

(In reply to comment #7)> The look very anonymous over here. See also bug 508819.Actually, it looks like the patch for bug 508819 that made getElementById() fail to return anonymous nodes was backed out (see comments in bug 522030).Now there's some HUDService code that depends on that behavior: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/console/hudservice/HUDService.jsm#4608bz: should we avoid depending on getElementById returning anonymous nodes (and fix the existing places where we do)?

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 10 • 14 years ago

Gah, bug 596752 :( Posting again:

(In reply to comment #7)
> The look very anonymous over here. See also bug 508819.

Actually, it looks like the patch for bug 508819 that made
getElementById() fail to return anonymous nodes was backed out (see comments in
bug 522030).

Now there's some HUDService code that depends on that behavior:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/console/hudservice/HUDService.jsm#4608

bz: should we avoid depending on getElementById returning anonymous nodes (and fix the existing places where we do)?

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 11 • 14 years ago

Comment on attachment 475738 [details] [diff] [review]
Patch v.2

>diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml

>-            b.setAttribute("flex", "1");

I don't think you want to remove this?

How about adding a gBrowser.getPanelForBrowser helper (it can just call getNotificationBox, or vice-versa) and making use of it in the places you're using .parentNode.parentNode?

>diff --git a/toolkit/components/console/hudservice/HUDService.jsm b/toolkit/components/console/hudservice/HUDService.jsm

>   getContentWindowFromHUDId: function HS_getContentWindowFromHUDId(aHUDId)

>+      var node = nodes[i];
>+
>+      if (node.localName == "stack" &&
>+          node.firstChild &&
>+          node.firstChild.contentWindow) {
>+        return node.firstChild.contentWindow;

so ugly :( "hud" objects should really have direct references to their associated <browser>. The whole HUD object hierarchy is horribly confusing though. File a followup?

	Justin Dolske [:Dolske] Assignee
	Comment 13 • 14 years ago

(In reply to comment #11)

> >-            b.setAttribute("flex", "1");
> 
> I don't think you want to remove this?

I did mean to, although it's harmless. I was reminded of bug 462113 comment 23.

	Justin Dolske [:Dolske] Assignee
	Comment 14 • 14 years ago

Per IRC, decided to skip adding getPanelForBrowser() for now, it only helped for these couple of cases, and overall felt kind of meh. Can always add it later if we think there's a good reason for it.

Filed bug 598270 on the HUD-<browser> linkage/cleanup.

Pushed http://hg.mozilla.org/mozilla-central/rev/e7fd6c393c98

	Boris Zbarsky [:bzbarsky]
	Comment 15 • 14 years ago

> bz: should we avoid depending on getElementById returning anonymous nodes 

Yes.  Both sicking and I have patches to make it not do that; we just didn't get to land them in time for 2.0.  This stuff _will_ break.  Don't rely on it.