52998 - menu in webpage does not work

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Frank Belew]

ON this page, there is a menu on the right column, second row
Choosing "Make your own in the US' causes the following to be output to the
terminal from which mozilla was run:
JavaScript error:
 line 0: lnav is not a function

Reproducability: always
Instructions: see above

The menu works in Netscape Communicator 4.75

Comment 1

[Scott Collins]

reproducible in a build from Friday night (Sept 15).

Comment 2

[Phil Schwartau]

Browser issue, not engine. Will attach two reduced HTML testcases,
one that reproduces the bug, and one where everything works fine.
The difference between the two is a single line in the HTML.

In the HTML that is buggy:  <input type=submit name="lnav"      value=go>
In the HTML that works:     <input type=submit name="lnavXXXXX" value=go>

Comment 3

[Phil Schwartau]

Attachment: HTML that reproduces bug (change the item in drop-down box)

Comment 4

[Phil Schwartau]

Attachment: HTML that works fine (change the item in drop-down box)

Comment 5

[Phil Schwartau]

Changing OS to "All", since the bug is the same on WinNT as on Linux. Using:

               Mozilla nightly 2000091505 on WinNT
               Mozilla nightly 2000091506 on Linux


Steps to reproduce: load an attachment, and change the item in the drop-down 
box. In the "bad" attachment, nothing will happen, and you will get this error 
in the debug console: 


JavaScript error: 
    line 0: object is not a function


In the good attachment, you will get no error, and a new Web page is loaded. 


The key is the JavaScript function lnav() in the HTML. It is being invoked
on the OnChange event of the <SELECT> element (the drop-down box.) It is also 
referred to by the <INPUT> element referred to above having the name "lnav".


NOTE: the <INPUT> element is contained within <NOSCRIPT> tags.

NOTE: both NN4.7 and IE 4.7 have no problem with either attachment above.
      They both work fine.


Sending to DOM Level 0 for further triage - 
                    

Comment 6

[Johnny Stenback (:jst)]

Having data inside noscript elements interfere with scripts could potentially
break even well designed pages out there, fixing this is easy and low risk,
gimme a + and I'll whip up a patch.

Comment 7

[Johnny Stenback (:jst)]

*** Bug 54735 has been marked as a duplicate of this bug. ***

Comment 8

[Nisheeth Ranjan]

Marking [rtm+ need info].  This can potentially affect existing sites that
duplicate names of elements inside noscript content.  This is a backwards
compatibility issue.

Comment 9

[Jason Eager]

Changed summary to be more specific and accurate (that it is a menu on a
webpage, NOT a XPFE menu that isn't displaying).

Comment 10

[Phil Schwartau]

*** Bug 54888 has been marked as a duplicate of this bug. ***

Comment 11

[Johnny Stenback (:jst)]

Attachment: Proposed fix

Comment 12

[vidur (gone)]

sr=vidur. This is the safest short-term fix, but the mechanism by which we deal
with <noscript> elements (and others of its ilk) should be changed post N6 RTM.
We're finding that these tags are being used as IFDEFs and that's not how our
current design thinks of them. Specifically, we try to build well-formed
subtrees of NOSCRIPT content and that won't produce compatible results with 4.x.

Comment 13

[Eric Pollmann]

r=pollmann  This fix has the lowest impact that will still fix the problem in
the test case and others like it.  As Vidur mentions, we should re-examine our
handling of content inside <noXXX> tags post-RTM, but this fix is safe and
reasonable for RTM.

Comment 14

[Johnny Stenback (:jst)]

Thanks for the reviews!

Comment 15

[Michael La Guardia]

marking rtm++

Comment 16

[Johnny Stenback (:jst)]

Fix checked into both netscape6 branch and trunk.

Comment 17

[Prashant Desale]

Re-opening this bug. I tested it with 2000-10-16-09 with Win-95 as well as 
Linux. I used Phil's testcases as well as URL mentioned above. For Phil's 
testcase I get error "line 0: Object is not a function".

Johnney I compared the fix you proposed [attachment] and the fix you checked in 
[I saw at bonsai.mozilla.org] and they differ. So may be that is causing 
problem.

Here is difference. I'm attaching only the smallest parts of code which differ.

YOUR PROPOSED FIX:
rv = MakeContentObject(aNodeType, nodeInfo, aForm, aWebShell,
-                           aResult, &content);
+                           aResult, &content, PRBool(mInsideNoXXXTag));

So here you proposed to delete line "aResult, &content);" and to add line 
"aResult, &content, PRBool(mInsideNoXXXTag));" 

But it is different that the actual code you checked in. CVS LOG shows 
following.

YOUR CHECKED IN CODE:
rv = MakeContentObject(aNodeType, nodeInfo, aForm, aWebShell,               
                       aResult, &content, !!mInsideNoXXXTag);

So here you added line "aResult, &content, !!mInsideNoXXXTag);".

New added line [aResult, &content, !!mInsideNoXXXTag);] is totally different 
than proposed line [aResult, &content, PRBool(mInsideNoXXXTag));]. I don't see 
PRBool function used at all. I'm gussing this is the problem.

But somehow this thing is not working.

Comment 18

[Johnny Stenback (:jst)]

Oh, I see the last change I made before attaching the patch didn't make it into
the patch. However, PRBool(mInsideNoXXXTag) is equal to !!InsideNoXXXTag in this
case so that's not it. I have no idea what caused this to regress tho.

This testcase works on the trunk today and harishd's uber noscript fix (bug
56501) will also fixes this regression so I think we don't need to do anything
here. Harishd's fix will be in todays branch builds so I'll test with that and
mark fixed again unless this still happens in todays branch builds...

Comment 19

[Johnny Stenback (:jst)]

Well, it works fine in todays branch and trunk build, so I'm marking FIXED again...

Comment 20

[Prashant Desale]

Yeah, it is working fine with 2000-10-18-10. Marking it verified.