Put <browser> in a <stack> so it's easy to overlay.

RESOLVED FIXED in Firefox 4.0b7

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

Trunk
Firefox 4.0b7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 475213 [details] [diff] [review]
Patch v.1

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.
Attachment #475213 - Flags: review?(gavin.sharp)
(Assignee)

Comment 1

7 years ago
Created attachment 475738 [details] [diff] [review]
Patch v.2

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?
Attachment #475213 - Attachment is obsolete: true
Attachment #475738 - Flags: review?(gavin.sharp)
Attachment #475213 - Flags: review?(gavin.sharp)
(Assignee)

Comment 2

7 years ago
Oh, actually, I think maybe we should, but instead as browser.linkedTabPanel?
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?
(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.
Huh? The <notificationbox>s aren't anonymous nodes.
Created attachment 475784 [details] [diff] [review]
linkedTabPanel getter

To be clear, something like this is what I was suggesting.
The look very anonymous over here. See also bug 508819.
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...
Attachment #475784 - Attachment is obsolete: true
(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)?
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)?
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?
Attachment #475738 - Flags: review?(gavin.sharp) → review+
Duplicate of this bug: 313190
(Assignee)

Comment 13

7 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.
Attachment #475738 - Flags: approval2.0+
(Assignee)

Comment 14

7 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
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b7
> 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.

Updated

6 years ago
Depends on: 624084
You need to log in before you can comment on or make changes to this bug.