Closed Bug 71191 Opened 24 years ago Closed 20 years ago

can't have binding on outermost (root) element in xml document

Categories

(Core :: XBL, defect)

Product:
Core
Component:
XBL
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: alex, Assigned: mrbkap)

References

Details

(Whiteboard: [xbl1.0])

Attachments

(4 files)

xbl file
185 bytes, text/xml
Details
Testcase
260 bytes, text/html
Details
patch v1
7.87 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
mochitestcases
7.26 KB, patch
bzbarsky
: review-
Details | Diff | Splinter Review 
This file fails to load:

---------------------------------------------------
-- test.xml --
<?xml version="1.0" ?>
<?xml-stylesheet href="test.css" type="text/css"?>
<mydoctag/>

---------------------------------------------------
-- test.css --
mydoctag{
	background-color: red;
	-moz-binding: url(test-binding.xml#mydoctag); }

---------------------------------------------------
-- test-binding.xml --
<?xml version="1.0"?>
<bindings xmlns="http://www.mozilla.org/xbl">
	<binding id="mydoctag">
        <handlers>
            <handler event="bindingattached">
                dump("bindingattached fired!\n");
            </handler>
        </handlers>
	</binding>
</bindings>

Neither is the background red, nor does the event handler fire.

If the binding is removed, behaviour is as expected (i.e. background is red).

If <mydoctag> is nested in some other tag, as in
<doc><mydoctag/></doc> 
the event handler fires ok.
accepting.  thanks.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → mozilla0.9.1
Whiteboard: [xbl1.0]
This isn't an XBL bug... the styles don't match.  Maybe you need to specify a 
namespace in the XML file?
Target Milestone: mozilla0.9.1 → Future
Blocks: zoompan
(In reply to comment #2)
> Maybe you need to specify a 
> namespace in the XML file?
Perhaps I didn't understood, but even with a XML file with namespace  we can't
add a binding on the root element...

(By the way, this is seen on linux also)
*** Bug 216263 has been marked as a duplicate of this bug. ***
Status: ASSIGNED → NEW
OS: Windows 2000 → All
QA Contact: jrgmorrison → ian
Hardware: PC → All
Summary: can't have binding on outermost element in xml document → can't have binding on outermost (root) element in xml document
Target Milestone: Future → ---
Attached file xbl file — 
Source of xbl file:
<?xml version="1.0"?>
<bindings xmlns="http://www.mozilla.org/xbl">
<binding id="test">
<content><children/> ***extra text caused by xbl binding***</content>
</binding>
</bindings>
Attached file Testcase — 
Source of html file:
<html><head>
<style>
html{background-color:green;
-moz-binding:url(http://bugzilla.mozilla.org/attachment.cgi?id=156915&action=view#test);}

</style>
</head>
<body>
After this there should come "***extra text caused by xbl binding***"
</body>
</html>

Currently you'll get to see a blank page, which isn't correct. This works when
I use the body as css selector.
The root node is Special (for example, you can't change its display type
either)....  So I do believe the frame construction path for the root never
looks for bindings.
For my patch in https://bugzilla.mozilla.org/show_bug.cgi?id=262166 I wanted to 
really add it to the top svg element (so the methods would be available in 
intrinsic events when there was no script element  in the document, but this 
blocked it. 

Would be nice to have this functionality.
Taking (I'm working on a fix).

Note to self: The top level canvas frame is always resized to the size of the
viewport after layout (and after being SetRect()'d to its child's size) by its
parent nsScrollBoxFrame (see nsScrollBoxFrame.cpp:369).
Assignee: hyatt → mrbkap
Attached patch patch v1 — — Splinter Review 
There were 3 seperate issues here:
1) nsCSSFrameConstructor::ConstructRootFrame() should not have been setting the
document element's primary frame, as this was stopping asynchronous bindings
from applying correctly.
2) nsCSSFrameConstructor::ContentInserted() needed to call AppendFrames() on
the root containing block if the initial reflow had already happened and the
root frame was being created due to, e.g., asycnrhonous XBL bindings.
3) CanvasFrame::Reflow() needed to invalidate its entire rect since the initial
reflow was missing the new styles applied.
Attachment #171775 - Flags: review?(bzbarsky)
Comment on attachment 171775 [details] [diff] [review]
patch v1

r+sr=bzbarsky
Attachment #171775 - Flags: superreview+
Attachment #171775 - Flags: review?(bzbarsky)
Attachment #171775 - Flags: review+
Fix checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
No longer blocks: zoompan
We should add this to a test suite.  I accidentally regressed it a few days ago...

We might need to get bug 379922 fixed first, though.
Flags: in-testsuite?
(In reply to comment #13)
> We should add this to a test suite.  I accidentally regressed it a few days
> ago...

Due to the patch for bug 377850, right? How did you determine that you regressed this bug? I can't seem to reproduce this bug with the patch in bug 377850 applied.
Odd.  I could...
Looks like this is broken on current trunk.
Attached patch mochitestcases — — Splinter Review 
Testcases for xhtml, xml, xul and svg.
In xml and svg, mochi stuff is not styled, because the class selector is not recognized there. It seems to me that should just work, no?
(In reply to comment #16)
> Looks like this is broken on current trunk.

It seems to work just fine for me with today's trunk build. Do you have an non-working example?
Attachment #269156 - Flags: review?(bzbarsky)
In my build, attachment 156917 [details] renders nothing at all.  Could be something on my end, though...
Ah, you're right. I'm getting this error in the error console.
Security Error: Content at https://bugzilla.mozilla.org/attachment.cgi?id=156917 may not load data from http://bugzilla.mozilla.org/attachment.cgi?id=156915&action=view#test.
This was caused by bug 379959, which might be intentional.
Is it really intentional that the failure of the binding to load causes the content to disappear?
Afaik, that has always been the case. I'm not sure if it's intentional.
I certainly don't like current behavior.
> In xml and svg, mochi stuff is not styled

See bug 386526.

> failure of the binding to load causes the content to disappear

That's how XBL1 basically works right now -- frame construction is blocked on the binding load, and if the binding load fails for any reason the frames are not constructed, because we trigger the reconstruct off completion of the binding load.

I don't know whether that's purposeful; you'd have to ask hyatt, I think.

Comment on attachment 269156 [details] [diff] [review]
mochitestcases

>Index: content/xbl/test/Makefile.in
>Index: content/xbl/test/test_bug71191.svg
>+      -moz-binding:url('#rd'); 

why "rd"?  Perhaps "root" would be clearer?

>+<body onload="doe()" xmlns="http://www.w3.org/1999/xhtml">

I'd prefer you didn't use an onload attribute here.  Use addLoadEvent() in the script below instead.

>+        ok(document.documentElement.bound, "Root element is xbl bound");

Hmm...  It seems that the ok() function is very easy to misuse (as in this case).  That call is missing the diagnostic message, or rather putting it in the "test name" field.

It would make more sense here to do:

  is(document.documentElement.bound, true,
     "Root element is xbl bound");

To make sure this doesn't pass by accident because SVG adds a "bound" property, could you have the same test without the binding comparing document.documentElement.bound to false?

>Index: content/xbl/test/test_bug71191.xhtml

This has a very different structure from the SVG test; in particular it does not wait for onload.  Why not?

All the comments from the SVG test apply here.

>Index: content/xbl/test/test_bug71191.xml

It looks like this test will time out if it fails, no?  Is there a reason for yet a third way to structure things like this?

>Index: content/xbl/test/test_bug71191.xul

Same comments as for SVG.
Attachment #269156 - Flags: review?(bzbarsky) → review-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: