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

RESOLVED FIXED in mozilla0.9.5

Status

SeaMonkey
General
--
enhancement
RESOLVED FIXED
16 years ago
13 years ago

People

(Reporter: Neil Pryde, Assigned: David Hyatt)

Tracking

Trunk
mozilla0.9.5
x86
All

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
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

Updated

16 years ago
Severity: normal → enhancement

Comment 1

16 years ago
*ducks* Hyatt, this is the bug needed for Multizilla. I hope you can find some 
time for it.
Marking NEW.
Assignee: asa → hyatt
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.6
(Reporter)

Comment 2

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

Neil
(Assignee)

Comment 3

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

Comment 4

16 years ago
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+ ??
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.5

Comment 5

16 years ago
Use an nsCOMPtr for innerStyleContext, and test it like a babulach. r=waterson
(Assignee)

Comment 6

16 years ago
Fixed by checkin to 49874.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 7

16 years ago
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?
(Assignee)

Comment 8

16 years ago
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".
(Assignee)

Comment 9

16 years ago
Not fixed.  Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

16 years ago
Somehow it still isn't shifting.  I'm not clear on what's going wrong.  WIll
have to investigate further.
Status: REOPENED → ASSIGNED
(Assignee)

Comment 11

16 years ago
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!
(Assignee)

Comment 12

16 years ago
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? :)
(Assignee)

Comment 13

16 years ago
Created attachment 50369 [details] [diff] [review]
Patch that ensures _content and content do the same thing.
(Assignee)

Comment 14

16 years ago
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.
Remove JSPROP_READONLY from JS_DefineUCProperty so that web content can override
_content if they want to. With that, sr=jst

Updated

16 years ago
Attachment #50369 - Flags: superreview+
(Assignee)

Comment 16

16 years ago
Consider it done.  Just need an r now.

Comment 17

16 years ago
r=blake
(Assignee)

Comment 18

16 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 19

16 years ago
It's the gift that keeps on giving!  Reopening a second time!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 20

16 years ago
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.
Status: REOPENED → ASSIGNED
(Assignee)

Comment 21

16 years ago
Created attachment 50445 [details] [diff] [review]
Fix appshell
Comment on attachment 50445 [details] [diff] [review]
Fix appshell

sr=ben@netscape.com
Attachment #50445 - Flags: superreview+
(Assignee)

Comment 23

16 years ago
Ok, hopefully this one takes. :)
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
hyatt: IIRC, window._content is deprecated anyway (in favour of window.content).
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.