Last Comment Bug 167493 - eHTMLTag_userdefined should be included as a valid head kid
: eHTMLTag_userdefined should be included as a valid head kid
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: All All
P1 normal (vote)
: mozilla1.4beta
Assigned To: harishd
: Moied
: Andrew Overholt [:overholt]
: 170413 (view as bug list)
Depends on:
Blocks: 170413 195260
  Show dependency treegraph
Reported: 2002-09-09 06:50 PDT by Thu-Van Luu
Modified: 2011-08-05 21:29 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (563 bytes, patch)
2002-09-09 07:56 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Splinter Review
Patch v1.1 (1.89 KB, patch)
2003-05-01 16:10 PDT, harishd
no flags Details | Diff | Splinter Review
Patch v1.2 (2.29 KB, patch)
2003-05-02 11:22 PDT, harishd
nisheeth_mozilla: review+
jst: superreview+
sspitzer: approval1.4b+
Details | Diff | Splinter Review

Description User image Thu-Van Luu 2002-09-09 06:50:10 PDT
User-Agent:       Mozilla/4.75 [en] (X11; U; Linux 2.2.12 i386)
Build Identifier: Mozilla/5.0 (X11;U; FreeBSD i386;en-US;rv:1.0rc1) Gecko/200020905

If I make my own XML element in the HTML <HEAD> of a frameset it isn't
recognized/created and I can't do any XBL binding to it...

<MyElement Help> </MyElement>

Reproducible: Always

Steps to Reproduce:
1.Load a HTML frameset document with a user defined element in the<HEAD>
2.Try to access the element or do some XBL binding...element wasn't created.

Actual Results:  

Expected Results:  
Created the user defined element in the DOM model and call the XBL constructor.
Comment 1 User image Randell Jesup [:jesup] 2002-09-09 07:25:29 PDT
Taking bug, will attach a patch

harishd: Could you rule on whether we're correct that userdefined should be
valid in <head>?
Comment 2 User image Randell Jesup [:jesup] 2002-09-09 07:56:47 PDT
Created attachment 98435 [details] [diff] [review]

Patch to add userdefined to the gHeadKids list
Comment 3 User image Randell Jesup [:jesup] 2002-09-09 07:57:32 PDT
Ready for review
Comment 4 User image harishd 2002-09-09 10:05:41 PDT
> 2.Try to access the element or do some XBL binding...element wasn't created.

That's not good. IMO, parser should not restrict any tag ( even userdefined ) from 
being a part of the content model. Also, userdefined tags have no specific rules
and hence MAY appear in the head context.
Comment 5 User image harishd 2002-09-09 10:07:01 PDT
Comment on attachment 98435 [details] [diff] [review]

Please add a one line comment with bug #. With that r=harishd
Comment 6 User image Randell Jesup [:jesup] 2002-09-11 16:16:56 PDT
I'll add a comment on the end with the bug #.  jst, sr?
Comment 7 User image Johnny Stenback (:jst, 2002-09-11 16:44:39 PDT
Comment on attachment 98435 [details] [diff] [review]


But please, diff -u next time!
Comment 8 User image Randell Jesup [:jesup] 2002-09-12 13:14:05 PDT
Fix checked in
Comment 9 User image rbs 2002-09-20 18:38:26 PDT
It seems that this patch had some issues. I traced what goes on when a
user-defined tag is outside the <HEAD> and saw something strange happening.
Suppose <MyElement> is inside the <BODY> instead of the <HEAD>. The code will
roll into HandleStartToken() and do what I illustrate below (traced in the

nsresult CNavDTD::HandleStartToken(CToken* aToken) {
const nsAString& tag = ((CStartToken*)aToken)->GetStringValue();
if (tag.Equals(NS_LITERAL_STRING("myElement")))

      PRBool theExclusive=PR_FALSE;
>>>with the patch, this *always* returns |theHeadIsParent=true| for a
>>>user-defined tag

#if 0
        //we'll move to this approach after beta...
      if(!isTokenHandled) {

        if(theHeadIsParent && (theExclusive || mOpenHeadCount>0)) {
        else result=HandleDefaultStartToken(aToken,theChildTag,theNode); 
        //the old way...
      if(!isTokenHandled) {
        if(theHeadIsParent || 
          (mOpenHeadCount>0  && (eHTMLTag_newline==theChildTag ||
eHTMLTag_whitespace==theChildTag))) {

>>>since |theHeadIsParent| is true, the smippet above
>>>will *always* add userdefined as a _leaf_ of the head

        else result=HandleDefaultStartToken(aToken,theChildTag,theNode); 
      //now do any post processing necessary on the tag...
  } //if

Comment 10 User image rbs 2002-09-23 16:41:29 PDT
Filed bug 170413 about the regression.
Comment 11 User image harishd 2002-09-24 09:56:51 PDT
May be all we need is 

if (!isTokenHandled) {
  if (theHeadIsParent || 
      (mOpenHeadCount > 0  && (eHTMLTag_newline == theChildTag    || 
                               eHTMLTag_whitespace == theChildTag ||
                               eHTMLTag_userdefined == theChildTag)) 
  else {
    result = HandleDefaultStartToken(aToken, theChildTag, theNode); 

Note: Patch ( id = 98435 ) should be reverted.
Comment 12 User image rbs 2002-09-26 03:00:27 PDT
Tried harishd's change but noted with the DOM inspector that it goes back to
square one, i.e, with the change, all userdefined tags get added as children of
the <body>.

Tracing in the debugger, I saw that something extra might be needed in
CNavDTD::OpenContainer(). Indeed, with the change, the code eventually rolls up
to CNavDTD::OpenContainer() and since userdefined isn't one of the special cases
that are there, it falls to the common codepath which pushes it as a child of
the <body>:

  if(!done) {
    result=(mSink) ? mSink->OpenContainer(*aNode) : NS_OK; 
Comment 13 User image Randell Jesup [:jesup] 2002-09-26 15:46:23 PDT
*** Bug 170413 has been marked as a duplicate of this bug. ***
Comment 14 User image Randell Jesup [:jesup] 2002-09-26 15:48:43 PDT
I'd like to keep the discussion here.  I'm going to back the patch out as
harishd suggested until we can find a solution.
Comment 15 User image Bob Clary [:bc:] 2002-11-13 05:29:47 PST
Randell, I am still seeing user defined tags inside body appearing as children
of the head.
shows this patch was not back out yet. 

This causes any XBL which uses user defined tags to fail. 
Comment 16 User image harishd 2002-11-13 16:47:51 PST
Have backed out the patch.
Comment 17 User image Bob Clary [:bc:] 2002-11-13 23:32:30 PST
Any chance of getting this backed out of 1.2 as well?
Comment 18 User image Randell Jesup [:jesup] 2002-11-14 21:39:39 PST
Sorry, I could have sworn I'd backed this out.  Thinking back, I think the tree
was closed at the time.  My apologies.  I'm asking for backout permission for 1.2
Comment 19 User image David Baron :dbaron: ⌚️UTC-8 2002-11-15 07:31:03 PST
a=dbaron for backing out of 1.2
Comment 20 User image Randell Jesup [:jesup] 2002-11-15 07:34:24 PST
Backed out of 1.2 branch
Comment 21 User image David Baron :dbaron: ⌚️UTC-8 2002-11-28 06:22:01 PST
It looks like the 1.2-branch backout was done incorrectly.  The 9 was not
changed to an 8.
Comment 22 User image David Baron :dbaron: ⌚️UTC-8 2002-11-28 06:30:51 PST
I believe this incorrect backout caused bug 182441 and bug 182348.
Comment 23 User image David Baron :dbaron: ⌚️UTC-8 2002-11-28 07:09:03 PST
I've moved tracking of the regressions to bug 182500.
Comment 24 User image harishd 2003-05-01 16:10:48 PDT
Created attachment 122246 [details] [diff] [review]
Patch v1.1

Treat the userdefined tags similar to whitespace such that it becomes a part of
the head context if there is an open head and becomes a part of the body
Comment 25 User image harishd 2003-05-01 17:48:01 PDT
FYI: Parser regression test PASSED. Also, ran the patch against the urls listed
under Debug->Verifications menu. URLs loaded fine.
Comment 26 User image harishd 2003-05-02 11:22:17 PDT
Created attachment 122313 [details] [diff] [review]
Patch v1.2

Same as patch v1.1 but with a mild clean up.
Comment 27 User image Johnny Stenback (:jst, 2003-05-02 12:08:17 PDT
Comment on attachment 122313 [details] [diff] [review]
Patch v1.2

Comment 28 User image Nisheeth Ranjan 2003-05-02 13:34:02 PDT
Comment on attachment 122313 [details] [diff] [review]
Patch v1.2

looks good.
Comment 29 User image harishd 2003-05-02 13:37:37 PDT
This fix is essential to unblock 195260 ( which is a serious bug ) and therefore
nominating for 1.4b.

Taking the bug.
Comment 30 User image (not reading, please use instead) 2003-05-02 13:43:05 PDT
Comment on attachment 122313 [details] [diff] [review]
Patch v1.2

Comment 31 User image harishd 2003-05-02 13:58:13 PDT
Fix landed.

Note You need to log in before you can comment on or make changes to this bug.