98109 - [RFE] make type="primary-content" dynamically shiftable

Status: RESOLVED FIXED

(SeaMonkey :: General, enhancement)

Description

[Neil Pryde]

Hyatt, if you can fix this that would be perfect for us.

This is what fabian wrote on one of our bug reports.

<quote>
I asked hyatt about this on #mozilla. Here's what he said. window._content
returns the window object of the _primary_ <browser>. A <browser> is primary if
it has the attribute type="primary-content". Only one <browser> should have that
type at a given time. All other <browser>'s should have type="content". The main
problem is that this type attribute is not dynamically shiftable, currently,
that is, the shells aren't live. Hyatt says "its easy" (for him) to make
type="primary-content" shiftable, thus updating the shells at the same time.
</quote>
/Neil

Comment 1

[Fabian Guisset]

*ducks* Hyatt, this is the bug needed for Multizilla. I hope you can find some 
time for it.
Marking NEW.

Comment 2

[Neil Pryde]

Hyatt, should this be done by making/changing some XLM/XBL bindings or do you
need to do this in C++??

Neil

Comment 3

[David Hyatt]

It's a C++ change.  I have this fixed and working on my other machine.  I'll get
a patch attached shortly.

Comment 4

[HJ]

David, how many votes do you need to get this in the tree befor 0.9.6? You might
know that we had over 20.000 downloads in September only. So this wouldn't be
hard to produce, but I actually don't like to force you into it this way. I,
more we, hope you will make it work, in the tree, way before 0.9.6 

David, can this actually be 'forced' into 0.9.4+ ??

Comment 5

[Chris Waterson]

Use an nsCOMPtr for innerStyleContext, and test it like a babulach. r=waterson

Comment 6

[David Hyatt]

Fixed by checkin to 49874.

Comment 7

[HJ]

David,

First of all, thanks, pleople would really love this improvement. Now, the
activated tab must use type="content-primary" and all other type="content" if
I'm correctly informed. BTW, is there anything else to do or is this all?

Comment 8

[David Hyatt]

Correct, so on a tab switch, you'll need to change the old tab's type attribute
to "content" and the new tab's attribute to "content-primary".

Comment 9

[David Hyatt]

Not fixed.  Reopening.

Comment 10

[David Hyatt]

Somehow it still isn't shifting.  I'm not clear on what's going wrong.  WIll
have to investigate further.

Comment 11

[David Hyatt]

Ok, I found the problem.  It looks like nsDOMClassInfo.cpp contains code that
when you first do a new resolve on a DOM window's script object, _content is
mapped to the content area, and this property is defined on the JS object.

This is what needs to change.  window._content is supposed to be fully dynamic,
which means that window._content can't point straight to a raw property, but
needs to invoke a getter function just as window.content did (and still does?)

If GlobalWindowImpl::GetContent were invoked for window._content, that would
solve the problem, since I'd hand back the correct content shell every time. 
This is what needs to happen, but I'm at a loss as to why window._content works
in this way.

jst, danm can you explain what's going on here and perhaps offer me guidelines
on how to tackle this? Thanks!

Comment 12

[David Hyatt]

I'm also very confused by the fact that this is still in nsIDOMWindowInternal's IDL.

/* [replaceable] content */
readonly attribute nsIDOMWindow                content;

I thought the whole point of switching to the disgusting window._content
notation was to avoid conflicts over having a window.content property defined,
and yet I see that it is still in fact defined.  What's going on here? :)

Comment 13

[David Hyatt]

Attachment: Patch that ensures _content and content do the same thing.

Comment 14

[David Hyatt]

Ok, this patch fixes the problem.  It makes window._content into a JS getter
instead of a raw prop value.  The JS getter than maps to window.content (which
is fully dynamic since it calls GetContent()).

Ready for r/sr.

Comment 15

[Johnny Stenback (:jst)]

Remove JSPROP_READONLY from JS_DefineUCProperty so that web content can override
_content if they want to. With that, sr=jst

Comment 16

[David Hyatt]

Consider it done.  Just need an r now.

Comment 17

[Blake Ross]

r=blake

Comment 18

[David Hyatt]

Fixed.

Comment 19

[David Hyatt]

It's the gift that keeps on giving!  Reopening a second time!

Comment 20

[David Hyatt]

This time there's trouble in xpfe/appshell land.  I bungled the logic for
properly nulling out and updating who the primary content area is.

Comment 21

[David Hyatt]

Attachment: Fix appshell

Comment 22

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 50445 [details] [diff] [review]
Fix appshell

sr=[e-mail]

Comment 23

[David Hyatt]

Ok, hopefully this one takes. :)

Comment 24

[Hixie (not reading bugmail)]

hyatt: IIRC, window._content is deprecated anyway (in favour of window.content).