We should stop using HTMLSpanElement for everything. See bug 843881 comment 4.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Can you please explain the bug in detail.
Sure. Right now, when you create an HTML element via createElement or via the parser, it can end up with HTMLSpanElement as its DOM interface, even if it's not a <span>. The elements this happens for are the ones whose entries in http://mxr.mozilla.org/mozilla-central/source/parser/htmlparser/public/nsHTMLTagList.h have "Span" as the second argument to HTML_TAG. So what this bug is about is going through those, figuring out what interface they should be using, and fixing them accordingly. For example, according to http://www.whatwg.org/specs/web-apps/current-work/multipage/obsolete.html#other-elements,-attributes-and-apis : The bgsound, isindex, multicol, nextid, rb, and spacer elements must use the HTMLUnknownElement interface. Presumably just replacing "Span" with "Unknown" for those elements will do the right thing. Plus adding tests, of course. For basefont the spec apparently calls for a separate interface; we'd need to create an interface and an implementation class for that..... or something. Given that we don't do anything useful with basefont elements, you could just map them to HTMLUnknownElement for now too. For "image" you probably want HTML_HTMLELEMENT_TAG() instead of HTML_TAG. Don't worry about keygen for now. That should cover all of them.
The tests will be there: content/html/content/test/test_bug389797.html
HTMLUnknownElement is also the default for unknown elements so maybe bgsound, insindex, et al can be handled that way. We should use HTMLUnknownElement for basefont too. We removed it from the codebase at some point to get it removed from HTML eventually.
> HTMLUnknownElement is also the default for unknown elements so maybe bgsound, insindex, > et al can be handled that way. That's possible too; might be higher risk patch. Good to hear about the basefont part of the spec being fiction! Wish the spec said so. ;)
I would like to work on this bug. Submitting a patch. Please check for any improvements.
Replaced HTML_TAG() for image with HTML_HTMLELEMENT_TAG() and removed the corresponding HTML_TAG() from the test file.
Comment on attachment 721081 [details] [diff] [review] Replaced "Span" with "Unknown" and made changes in the test file Review of attachment 721081 [details] [diff] [review]: ----------------------------------------------------------------- Quick drive-by comment. ::: content/html/content/test/test_bug389797.html @@ -158,5 @@ > HTML_TAG("html", "Html"); > HTML_TAG("i", ""); > HTML_TAG("iframe", "IFrame", [ "nsIDOMGetSVGDocument", "nsIDOMMozBrowserFrame" ], > [ "nsIFrameLoaderOwner" ]); > -HTML_TAG("image", "Span"); shouldn't that be: HTML_TAG("image", "");
Made changes suggested by Mounir.
Comment on attachment 721118 [details] [diff] [review] Replaced "Span" with "Unknown" and made changes in the test file This looks reasonable. Can you also add a test that makes sure that those elements end up with the right prototype when parsed or created via createElement?
Attachment #721118 - Flags: feedback?(bzbarsky) → feedback+
FWIW, I think it is preferable to not use bug numbers for test names but real names. The only cases where bug numbers could make sense is when this is really testing an edge case that was found in a bug report. I know most tests use bug numbers but they are most of the time wrong to do so ;)
After running the mochi-test on the build with all the changes, there seems to be many fail cases http://www.pastebin.mozilla.org/2211811
Changed test file name to |test_element_prototype.html| as suggested by mounir. Made a few changes to |test_element_prototype.html| which resolved the fails on mochitest. Now mochitest throws some errors which are based on this line and the lines below it http://mxr.mozilla.org/mozilla-central/source/content/html/content/test/test_bug389797.html?force=1#247 What can be done to resolve these errors? Remove those test code? http://www.pastebin.mozilla.org/2212114 (mochitest errors)
I recommend updating the tests for the new pass condition and maybe adding a pointer to this bug.
> http://www.pastebin.mozilla.org/2212114 (mochitest errors) Is that with attachment 723959 [details] [diff] [review]? Because if so, something is broken....
http://hg.mozilla.org/mozilla-central/file/519c7cf5605b/content/html/content/test/test_bug389797.html#l247 needs to be adjusted for cases when classInfoString is "HTMLUnknownElement". Also, the other two tests that use HTML_TAG should be fixed.
Made the suggested changes and resolved all mochitest errors.
Comment on attachment 724014 [details] [diff] [review] Replaced "Span" with "Unknown" and added a test file Looks good. What about the rest of comment 17? Also, it's probably better to have the commit comment actually describe what you're doing and include the reviewer info.
Attachment #724014 - Flags: feedback?(bzbarsky) → feedback+
There you go! Hope I didn't miss anything this time :)
Comment on attachment 724060 [details] [diff] [review] Replaced "Span" with "Unknown" and added a test file r=me
Attachment #724060 - Flags: review?(bzbarsky) → review+
Assignee: nobody → indiasuny000
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
So why isn't 'image' Unknown?
Because it's being dealt with in bug 843881. No point thrashing it here....
I've added this bug to the compatibility doc. Please correct the info if wrong. https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_22 Also, please update the following doc: https://developer.mozilla.org/en-US/docs/DOM/document.createElement
7 years ago
<sigh>. Fixed that just like I fixed the changes from comment 26... Also fixing the summary to make it clearer what changed.
Summary: Stop defaulting to HTMLSpanElement → Stop defaulting to HTMLSpanElement for bgsound, multicol, image
Summary: Stop defaulting to HTMLSpanElement for bgsound, multicol, image → Stop usign the HTMLSpanElement interface for bgsound, multicol, image
Summary: Stop usign the HTMLSpanElement interface for bgsound, multicol, image → Stop using the HTMLSpanElement interface for bgsound, multicol, image
6 years ago
You need to log in before you can comment on or make changes to this bug.