Closed
Bug 302513
Opened 20 years ago
Closed 19 years ago
Figure out id attributes inside repeats
Categories
(Core Graveyard :: XForms, defect)
Core Graveyard
XForms
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: allan, Assigned: allan)
References
()
Details
(Keywords: fixed1.8, testcase)
Attachments
(4 files, 5 obsolete files)
Right now we just kill the id attribute on cloned content:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/xforms/nsXFormsRepeatElement.cpp&rev=1.21&root=/cvsroot&mark=923#920
I think the idea is that the id should be valid for the currently selected index
inside a repeat. And stuff inside "repeat rows" should use have a "local scope"
for id's.
"Inspiration"-links :)
http://lists.w3.org/Archives/Member/w3c-forms/2005JanMar/att-0181/2005Feb28.html#topic6
http://lists.w3.org/Archives/Member/w3c-forms/2005JanMar/0182
Assignee | ||
Comment 1•20 years ago
|
||
Here's a switch inside a repeat. People would except all the switches to work I
guess...
Assignee | ||
Comment 2•20 years ago
|
||
*** Bug 302520 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•19 years ago
|
Assignee: aaronr → allan
Status: ASSIGNED → NEW
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•19 years ago
|
||
Attachment #190845 -
Attachment is obsolete: true
Assignee | ||
Comment 5•19 years ago
|
||
Assignee | ||
Comment 6•19 years ago
|
||
Here's a patch handling "local scope" for setfocus and toggle, passing the two
attached testcases.
I need to do a better explanation of what I've done, and handle dispatch too.
It should behave in the same way.
Assignee | ||
Comment 7•19 years ago
|
||
Assignee | ||
Comment 8•19 years ago
|
||
Also tests one more thing: Dispatch element behaviour in non-focused repeat
row: Load the test-case and press the add button in the group at the bottom of
the form.
Attachment #193059 -
Attachment is obsolete: true
Assignee | ||
Comment 9•19 years ago
|
||
Here's a patch for the elements that should have special handling for use with
repeats.
The idea is that xforms actions using ids but residing outside a repeat should
work on the currently selected repeat row. Actions inside repeats should work
on the element matching the id inside their own repeat row (ie. scoped).
I've implemented a nsXFormsUtils::GetElementById() that checks if an element
with a given id is a child of a repeat. If it is, it checks whether the action
element is inside a repeat and then uses the action element's repeat row, if
not it uses the currently focused repeat row.
It's not exactly cheap :( We could possibly use the mRepeatState mechanisms
used for nsXFormsDelegateStub, but I'd rather add that later if we agree on
that.
Attachment #193054 -
Attachment is obsolete: true
Attachment #193071 -
Flags: review?(smaug)
Comment 10•19 years ago
|
||
Comment on attachment 193071 [details] [diff] [review]
Patch for toggle, setfocus, and dispatch
>+ }
>+ NS_NAMED_LITERAL_STRING(idStr, "id");
>+ PRUint32 i;
>+ nsAutoString idVal;
>+ nsCOMPtr<nsIDOMNode> childNode;
>+ for (i = 0; i < elements; ++i) {
>+ descendants->Item(i, getter_AddRefs(childNode));
>+ element = do_QueryInterface(childNode);
>+ if (element) {
>+ element->GetAttribute(idStr, idVal);
>+ if (idVal.Equals(aId)) {
>+ break;
This is wrong. Attribute called "id" may not actually be the ID attribute.
You'll have to QI to nsIContent and use GetIDAttributeName().
Attachment #193071 -
Flags: review?(smaug) → review-
Assignee | ||
Comment 11•19 years ago
|
||
(In reply to comment #10)
> (From update of attachment 193071 [details] [diff] [review] [edit])
> This is wrong. Attribute called "id" may not actually be the ID attribute.
> You'll have to QI to nsIContent and use GetIDAttributeName().
I guess you are right. How about all the other places we use "id", shouldn't
they be fixed too?
Comment 12•19 years ago
|
||
(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 193071 [details] [diff] [review] [edit] [edit])
> > This is wrong. Attribute called "id" may not actually be the ID attribute.
> > You'll have to QI to nsIContent and use GetIDAttributeName().
>
> I guess you are right. How about all the other places we use "id", shouldn't
> they be fixed too?
No. I checked them and they are all related to XForms elements, and in those
cases the "id" attribute is actually ID.
Assignee | ||
Comment 13•19 years ago
|
||
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (From update of attachment 193071 [details] [diff] [review] [edit] [edit] [edit])
> > > This is wrong. Attribute called "id" may not actually be the ID attribute.
> > > You'll have to QI to nsIContent and use GetIDAttributeName().
> >
> > I guess you are right. How about all the other places we use "id", shouldn't
> > they be fixed too?
>
> No. I checked them and they are all related to XForms elements, and in those
> cases the "id" attribute is actually ID.
Why? The id attribute is not defined anywhere in the XForms spec. The id
attribute comes from the host language.
"A host language must permit an attribute of type xsd:ID on each XForms element."
[http://www.w3.org/TR/xforms/slice3.html#structure-attrs-common]
Assignee | ||
Comment 14•19 years ago
|
||
Ok, now using GetIDAttributeName. I also use GetNodeType to check for whether
it is an element, must be cheaper than QI.
Attachment #193071 -
Attachment is obsolete: true
Attachment #193131 -
Flags: review?(smaug)
Comment 15•19 years ago
|
||
(In reply to comment #13)
> Why? The id attribute is not defined anywhere in the XForms spec. The id
> attribute comes from the host language.
> "A host language must permit an attribute of type xsd:ID on each XForms element."
> [http://www.w3.org/TR/xforms/slice3.html#structure-attrs-common]
>
Ah, you're right. I wonder how to decide what is actually the host language, if
the document contains elements from several "languages"
But in our case, the ID attribute is always "id", that is harcoded in XTF :(
Comment 16•19 years ago
|
||
Comment on attachment 193131 [details] [diff] [review]
Patch using GetIDAttributeName()
>+
>+ PRUint32 i;
>+ PRUint16 type;
>+ nsAutoString idVal;
>+ nsCOMPtr<nsIDOMNode> childNode;
>+ for (i = 0; i < elements; ++i) {
>+ descendants->Item(i, getter_AddRefs(childNode));
>+ childNode->GetNodeType(&type);
>+ if (type == nsIDOMNode::ELEMENT_NODE) {
>+ nsCOMPtr<nsIContent> content(do_QueryInterface(childNode));
>+ NS_ASSERTION(content, "An ELEMENT_NODE not implementing nsIContent?!");
>+ content->GetAttr(kNameSpaceID_None,
>+ content->GetIDAttributeName(),
>+ idVal);
>+ if (idVal.Equals(aId)) {
Could you do
rv = content->GetAttr(kNameSpaceID_None,
content->GetIDAttributeName(),
idVal);
if (rv == NS_CONTENT_ATTR_HAS_VALUE && idVal.Equals(aId)) {
I think that might be a bit faster.
With that r=me
Attachment #193131 -
Flags: review?(smaug) → review+
Assignee | ||
Comment 17•19 years ago
|
||
(In reply to comment #15)
> (In reply to comment #13)
> > Why? The id attribute is not defined anywhere in the XForms spec. The id
> > attribute comes from the host language.
> > "A host language must permit an attribute of type xsd:ID on each XForms
element."
> > [http://www.w3.org/TR/xforms/slice3.html#structure-attrs-common]
>
> Ah, you're right. I wonder how to decide what is actually the host language, if
> the document contains elements from several "languages"
Frankly, I do not know. It's been discussed in the WG a couple of times just to
include the id attribute in XForms instead. The above leads to problems again
and again.
> But in our case, the ID attribute is always "id", that is harcoded in XTF :(
:(
Assignee | ||
Comment 18•19 years ago
|
||
(In reply to comment #16)
> Could you do
> rv = content->GetAttr(kNameSpaceID_None,
> content->GetIDAttributeName(),
> idVal);
> if (rv == NS_CONTENT_ATTR_HAS_VALUE && idVal.Equals(aId)) {
>
> I think that might be a bit faster.
Right on! I was thinking of HasAttr() but thought it was too slow, forgot that
the return value actually has the info.
Assignee | ||
Comment 19•19 years ago
|
||
Attachment #193131 -
Attachment is obsolete: true
Attachment #193141 -
Flags: review?(doronr)
Updated•19 years ago
|
Attachment #193141 -
Flags: review?(doronr) → review+
Assignee | ||
Updated•19 years ago
|
Comment 21•19 years ago
|
||
Removing xf-to-branch.
Updated•19 years ago
|
Whiteboard: xf-to-branch
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•