Closed
Bug 324378
Opened 19 years ago
Closed 19 years ago
[FIXr]Attribute setting isn't always "first wins"
Categories
(Core :: DOM: Core & HTML, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
()
Details
(Keywords: compat)
Attachments
(1 file)
|
5.11 KB,
patch
|
sicking
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
The net result of the changes for bug 213347 and bug 214577 is that in some cases the first attr doesn't win. Example is in the URL bar -- none of the attributes in DOM inspector should have the value "b".
Fix coming up.
|
Assignee | |
Comment 1•19 years ago
|
||
|
|
Assignee | |
Comment 2•19 years ago
|
||
Attachment #209333 -
Flags: superreview?(jst)
Attachment #209333 -
Flags: review?(bugmail)
|
||
Comment 3•19 years ago
|
||
Comment on attachment 209333 [details] [diff] [review]
Fix
sr=jst
Attachment #209333 -
Flags: superreview?(jst) → superreview+
|
|
Assignee | |
Updated•19 years ago
|
Summary: Attribute setting isn't always "first wins" → [FIX]Attribute setting isn't always "first wins"
Comment on attachment 209333 [details] [diff] [review]
Fix
> case eHTMLTag_head:
> rv = OpenHeadContext();
> if (NS_SUCCEEDED(rv)) {
>- rv = AddAttributes(aNode, mHead, PR_FALSE, PR_TRUE);
>+ rv = AddAttributes(aNode, mHead, PR_FALSE, mHaveSeenHead);
>+ mHaveSeenHead = PR_TRUE;
You could always use PR_TRUE here, it would be slightly slower, but there's not usually that many head attributes. That way mHaveSeenHead isn't needed.
> }
> break;
> case eHTMLTag_body:
> rv = OpenBody(aNode);
> break;
> case eHTMLTag_html:
> if (mRoot) {
>- AddAttributes(aNode, mRoot, PR_TRUE, PR_TRUE);
>+ // If we've already hit this code once, need to check for
>+ // already-present attributes on the root.
>+ AddAttributes(aNode, mRoot, PR_TRUE, mNotifiedRootInsertion);
Same here, though we already do have the member so less of an advantage here.
r=me
Attachment #209333 -
Flags: review?(bugmail) → review+
|
|
Assignee | |
Comment 5•19 years ago
|
||
> You could always use PR_TRUE here, it would be slightly slower
It would also reverse the order of attributes on <head> every single round trip through the editor. I thought that undesirable. ;)
> Same here
Yeah, same issue here.
Reversing the attrs on the second and later nodes seems ok to me since it's a one-time fixup. But I didn't want to break round-tripping in general.
Makes sense
|
|
Assignee | |
Updated•19 years ago
|
Summary: [FIX]Attribute setting isn't always "first wins" → [FIXr]Attribute setting isn't always "first wins"
|
|
Assignee | |
Comment 7•19 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 19 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
|
||
Comment 9•15 years ago
|
||
The Mochitest for this bug fails with the HTML5 parser, because the HTML parser doesn't add the attributes of a second <head> tag to the head element.
The spec aligns with IE and Safari here; they don't add the attributes, either. Opera adds the attributes of a second <head> tag to the head element.
I tried to find the rationale for adding the attributes of a second <head> to the head element. The explicit code for doing that specifically for head was introduced by vidur without a bug number in
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/html/document/src/nsHTMLContentSink.cpp&rev=3.182
Do we want to keep the old behavior here (and get the spec changed) or shall I just weaken the test case not to test attributes of the head element? I favor weakening the test case and adopting the IE/Safari behavior, because it's hard to believe this to be a Web compat issue if IE and Safari get away with not implementing the special case.
|
|
Assignee | |
Comment 10•15 years ago
|
||
I would be fine with that change.
Agreed
|
|
||
Comment 12•15 years ago
|
||
OK. The change is tracked as bug 539882.
|
||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•