XUL - nsXULElement::GetStyle() not implemented

RESOLVED FIXED

Status

()

defect
P3
normal
RESOLVED FIXED
20 years ago
11 years ago

People

(Reporter: hangas, Assigned: dbaron)

Tracking

({helpwanted})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [xul1.0][patch])

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

20 years ago
Using a box in a XUL file:
<box id="cvbName" align="vertical" class="CardView">
Unable to set the style on this box from JavaScript with the line:
node.style.display = "none";
The following error appears in the output window:
JavaScript error: node.style has no properties
however this does work fine with <html:div>

This is blocking Address Book UI development.

Updated

20 years ago
Assignee: trudelle → evaughan

Comment 1

20 years ago
reassigning to evaughan

Updated

20 years ago
Assignee: evaughan → hyatt

Comment 2

20 years ago
David,

This is a XUL thing. How would you do this?

Updated

20 years ago
Status: NEW → ASSIGNED
Target Milestone: M7

Comment 3

20 years ago
This isn't even implemented in our content model, seeing as how the STYLE
attribute is a property of nsIDOMHTMLElement (and therefore doesn't exist in
XUL).  I'm investigating what it will take to duplicate the HTML code over on
the XUL side (sigh).

Updated

20 years ago
Target Milestone: M7 → M8

Updated

20 years ago
Target Milestone: M8 → M9

Comment 4

20 years ago
Slipping to M9.  This is not causing a crash or making someone's life miserable,
and so (given my current level of doomage), this can slip.

Comment 5

20 years ago
*** Bug 8797 has been marked as a duplicate of this bug. ***

Comment 6

20 years ago
Moving to M10.

Comment 7

20 years ago
*** Bug 12474 has been marked as a duplicate of this bug. ***

Updated

20 years ago
Assignee: hyatt → waterson
Status: ASSIGNED → NEW

Comment 8

20 years ago
All yours, baby.
Can I assume this will be fixed before beta for purposes of demos/documentation?

Updated

20 years ago
Status: NEW → ASSIGNED
Target Milestone: M11 → M15

Comment 10

20 years ago
Probably not. I'd rather not implement this before beta; after beta we're going
to unify XUL and HTML ocntent model code. At which point we'll get this for
free. If there is a compelling reason that we need this for some functionality
before beta, somebody let me know.

Comment 11

20 years ago
giving me rest of phillips open qa contact bugs, sorry for spam

Comment 12

20 years ago
*IGNORE* - more massive spam, changing open XPToolkit bug's QA contact to
jrgm@netscape.com

QA Contact: paulmac → jrgm

Comment 13

20 years ago
*IGNORE* - massive spam changing open XPToolkit bug's QA contact to
jrgm@netscape.com

Comment 14

20 years ago
doesn't this work now?
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED

Comment 16

20 years ago
It looks like this is not implemented. The attachment is a simple test
which toggles style.display = 'none|block' for html:div and xul:box. 
The xul:box causes this error in the console:

JavaScript Error: uncaught exception: [Exception... "Method not implemented"  
code: "-2147467263" nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: 
"http://bugzilla.mozilla.org/showattachment.cgi?attach_id=7086 Line: 12"]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 17

20 years ago
Doh! nsXULElement::GetStyle() is most certainly unimplemented. Darn.

jst, pierre: how would you feel about me making nsDOMCSSDeclaration objects 
"publicly accessable" from layout via XPCOM factory method? I'd like to 
construct one of these and then wrap it to provide an 
nsDOMCSSAttributeDeclaration object for XUL elements.
Status: REOPENED → ASSIGNED
Target Milestone: M15 → M16

Comment 18

19 years ago
Changing summary from "XUL - Unable to set box.style from JavaScript' to the 
general 'XUL - nsXULElement::GetStyle() not implemented'
Summary: XUL - Unable to set box.style from JavaScript → XUL - nsXULElement::GetStyle() not implemented

Comment 19

19 years ago
waterson (in response to your question from 2000-04-02): the thing that bothers 
me is that nsDOMCSSDeclaration comes with all the nsIDOMCSS2Properties while the 
interface you need (I think) is just the nsIDOMCSSStyleDeclaration.

Comment 20

19 years ago
Ok, so what do you suggest?
Pierre, CSS2Properties currently inherits from CSSStyleDeclaration in mozilla
but it *doesn't* do that in the current DOM Level 2 CSS spec, I was wondering
about why it does that and I think we should change that to comply with the DOM2
spec, unless there's a really good reason to have it like it is today.

Does anyone know of a good reason for having CSS2Properties inherit from
CSSStyleDeclaration, or would we gain something from *removing* the inheritance
between the interfaces? (not that I know if it's at all related to this bug tho)
What the DOM2 spec does say is:

  A compliant implementation is not required to implement the CSS2Properties
  interface. If an implementation does implement this interface, the expectation
  is that language-specific methods can be used to cast from an instance of the
  CSSStyleDeclaration interface to the CSS2Properties interface.

which I think explains the current implementation in Mozilla.  See
http://www.w3.org/TR/DOM-Level-2/css.html#CSS-CSS2Properties
David, FYI:

... the expectation is that language-specific methods can be used to cast from
an instance of the CSSStyleDeclaration interface to the CSS2Properties
interface.

Means that you're supposed to get at the CSS2Properties interface by doing
QueryInreface in C++ or some automatic typecasting in JS, on the object that
implements CSSStyleDeclaration if it's supported. It does not mean that
CSS2Properties needs to, nor should, *inherit* from the CSSStyleDeclaration
interface.

We can, and IMO should, follow the DOM spec on this by changing the interface
declaration, the only downside I can see is that we'll end up with one more
vtable entry in nsDOMCSSDeclaration (ie 4 bytes more per instance).

Comment 24

19 years ago
not nsbeta2, move to M17
Target Milestone: M16 → M17

Updated

19 years ago
Keywords: helpwanted
Target Milestone: M17 → Future

Comment 25

19 years ago
This bug might be a blocker for alphanumerica's tool.  cc'ing johng and hangas
Actually, i've come to the conclusion that this bug might not be exactly what i
need. I need to set pseudo style rules on xul elements. I'm not worried about
doing gets right now because rules and settings for the app are stored in class
arrays. 

I need to set hover, active etc . .  . Some way.

I also talked to Waterson about this and i'm not sure there is anything around
that can do this yet.

I am going to file a new bug since i beleive it is a different problem from this
one.

pete
Pete, FYI: it sounds like the DOM Level 2 DocumentCSS method getOverrideStyle()
would do what you're looking for, it's unfortunately not implemented in mozilla
right now but implementing it shouldn't be too hard...

Comment 28

19 years ago
getOverrideStyle() ?

Is :hover, :active considered override styles?

I need to "set" more than i need "get".

If so then yes that is what i am looking for.

thanks

pete

Comment 30

19 years ago
I'm trying to use display = none to make a fullscreen mode in mozilla, and this
is keeping me from doing it.  I want to avoid using hidden = true as that would
mean caching previous values, and i don't want to have to implement that if
possible.  Any chance of movement on this?

Comment 31

18 years ago
XUL 1.0 requirement.
Assignee: waterson → hyatt
Status: ASSIGNED → NEW
Target Milestone: Future → mozilla0.9.6

Updated

18 years ago
Blocks: 70753
Status: NEW → ASSIGNED
Whiteboard: [xul1.0]

Updated

18 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.8

Comment 32

18 years ago
*** Bug 109759 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Attachment #7086 - Attachment mime type: text/xul → application/vnd.mozilla.xul+xml

Updated

18 years ago
Target Milestone: mozilla0.9.8 → mozilla0.9.9

Comment 33

18 years ago
*** Bug 120762 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Target Milestone: mozilla0.9.9 → mozilla1.0

Comment 34

18 years ago
Adding keyword nsbeta1 to several bugs.
Keywords: nsbeta1

Comment 35

18 years ago
nsbeta1- per Nav triage team
Keywords: nsbeta1nsbeta1-

Comment 36

18 years ago
Moving non nsbeta1+ XUL 1.0 bugs to mozilla1.2
Target Milestone: mozilla1.0 → mozilla1.2

Updated

18 years ago
No longer blocks: 70753
*** Bug 145927 has been marked as a duplicate of this bug. ***

Comment 38

17 years ago
--> default owner
Assignee: hyatt → jaggernaut
Status: ASSIGNED → NEW
Target Milestone: mozilla1.2alpha → ---

Comment 39

17 years ago
I suggest this bug be assigned to XP Toolkit/Widgets: XUL, and also set All/All.

Updated

17 years ago
Component: XP Toolkit/Widgets → XP Toolkit/Widgets: XUL
OS: Mac System 8.5 → All
Hardware: Macintosh → All

Comment 40

16 years ago
Can anyone please explain to me why implementing the style attribute for XUL is
different then implementing it for HTML and SVG?

It is probably needless to say that it makes dynamic changes to XUL element
appearence almost impossible.

Comment 41

16 years ago
A limited workaround - use the insertRule and deleteRule method of a style sheet
for example document.styleSheets[1].insertRule('label {background-color:red}',0)
Posted patch patch (obsolete) β€” β€” Splinter Review
This is almost all of what's necessary.  The one problem I know about is that
nsXULElement::SetInlineStyleAttribute doesn't do all the stuff that
SetAttribute does -- so XBL inheriting the style attribute, or things like
that, won't work.  It could also use a little more than the cursory testing
I've done so far.
Oh, also, nsXULElement stores the string for the style attribute, and this
doesn't keep that up-to-date.  I could fix both problems at once by just calling
|SetAttr|, but that would waste work re-parsing the string, and might confuse
nsDOMCSSAttrDeclaration too.
Taking.
Assignee: jag → dbaron
Whiteboard: [xul1.0] → [xul1.0][patch]
Posted patch patch β€” β€” Splinter Review
Attachment #132783 - Attachment is obsolete: true
Comment on attachment 133531 [details] [diff] [review]
patch

I discussed this with hyatt in person, and he seemed happy.

There's a bit of changing types nsIStyleRule->nsICSSStyleRule and
nsIHTMLContent->nsIStyledContent.

Beyond that, the main interesting changes are in nsXULElement.cpp.  The
nsXULAttributes struct, for a fully-faulted nsXULElement, is increased by a
word.  We fully fault XUL elements whenever the .style property is accessed.

XUL, unlike HTML, keeps the string version and the style rule version of the
attribute, so we have to keep them in sync when it's set via element.style. 
Thus some of SetAttr() is duplicated, and some is refactored.  Probably some of
the stuff I do in nsXULElement::SetInlineStyleRule could be conditioned on
|aNotify| or on the presence of mutation events, but I suspect the performance
costs of the things I do there are negligible enough that it won't matter.
Attachment #133531 - Flags: superreview?(bzbarsky)
Attachment #133531 - Flags: review?(bzbarsky)
Oh.  I forgot to turn the two "XXX" comments in nsXULElement::SetInlineStyleRule
into non-"XXX" comments.
Comment on attachment 133531 [details] [diff] [review]
patch

>Index: xul/content/src/nsXULElement.cpp

>+    // XXX Need to fix the copy stored as a string too!

Why is this an XXX comment?  Do we want to stop doing that at some point?  Or
just come up with a better way to do it?  Maybe clarify?

>+        nsCOMPtr<nsINodeInfo> ni;
>+        nimgr->GetNodeInfo(nsXULAtoms::style, nsnull, kNameSpaceID_None,
>+                           getter_AddRefs(ni));

This call could fail....

With those nits picked, r+sr=bzbarsky.	It's a good thing we have immutable
style rules, eh?  What with that prototype stuff... ;)
Attachment #133531 - Flags: superreview?(bz-vacation)
Attachment #133531 - Flags: superreview+
Attachment #133531 - Flags: review?(bz-vacation)
Attachment #133531 - Flags: review+
Fix checked in to trunk, 2003-10-29 17:40 -0800.

Note that it's not advisable for XUL authors concerned about performance /
memory use to use this extensively since it causes the XUL element to become
fully heavyweight (which is the same memory cost as setting an attribute through
the DOM).
Status: NEW → RESOLVED
Closed: 20 years ago16 years ago
Resolution: --- → FIXED
No DOM Inspector changes required, right?

Updated

11 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.