Closed Bug 191579 Opened 21 years ago Closed 21 years ago

<script EVENT="onload()" FOR="window" LANGUAGE="JavaScript"> doesn't execute

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.3final

People

(Reporter: ash, Assigned: jst)

References

(
URL
)

Details

(Keywords: regression, Whiteboard: [HAVE FIX])

Attachments

(2 files, 1 obsolete file)

real content of page you get stuck on
964 bytes, text/html
Details
Treat <script for='window' event='onload()'> as inline script.
3.27 KB, patch
bzbarsky
: review+
peterv
: superreview+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021212
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021212

the state farm bank login page serves the html i'll attach in a moment as part
of the login process.  the script tag looks like this:
<script EVENT="onload()" FOR="window" LANGUAGE="JavaScript">
The javascript inside that tag isn't executed, which causes the login process to
fail.  If EVENT and FOR are removed, the javascript is executed.

The attachment is the entire page served, so it's quite malformed, but that's
not the problem i don't think.  it's something to do with the EVENT and FOR attribs.

Reproducible: Always

Steps to Reproduce:
1. load statefarm.com/account.htm
2. log in as a valid state farm user

Actual Results:  
You get stuck on a blank page with the content i'll attach.

Expected Results:  
mozilla should have executed the javscript in the blank page, which includes
setting some cookies and redirecting to a different page.

    
    

    
  
Comment on attachment 113308 [details]
content of the page where you get stuck after logging in

this attachment had some debugging output in it i added to see what was going
on.  original content to follow as new attachment

        Attachment #113308 -
        Attachment is obsolete: true

    
    

    
  


  

    
    

    
  
Confirming.  We need to not treat "window" as an id....
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression

    
    

    
  
Geez, what a mess.

    
    

    
  
Looks like IE is special casing <script for=window event=onload> to treat that
as the onload handler of the webpage, but from what I can tell, that's the only
window event that can be registerd with a <script for=window event=...> element.
I *really* don't want to support this in Mozilla, we never did, I'm guessing the
reason this worked in older builds was that Mozilla executed a <script
for=window event=onload> element as inline script, but that's wrong too. It
needs to be registerd as the onload handler for the window, and it overrides
<body onload='...'> too, not cool.

We could either make an attempt at emulating IE's messy behavior here, or we
could revert back to what we used to do for this special case only. Thoughts?

    
    

    
  
Hmm... I'm tempted to just execute it as an inline script, since that's much
simpler.....

    
    

    
  


  
I tried my best to reverse-engineer how IE parses these attributes, and this is
what I got. Thourhgts?

    
    

    
  
Er, "Thoughts?", that is...

    
  

        Attachment #113763 -
        Flags: review?(bzbarsky)

    
  
Status: NEW → ASSIGNED
OS: Linux → All
Priority: -- → P2
Hardware: PC → All
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla1.3final

    
    

    
  
Comment on attachment 113763 [details] [diff] [review]
Treat <script for='window' event='onload()'> as inline script.

> +  // We foud for="window", now check for event="onload".

"found".

This looks pretty reasonable.  ;)

        Attachment #113763 -
        Flags: review?(bzbarsky) → review+

    
  

        Attachment #113763 -
        Flags: superreview?(peterv)

    
    

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

    
    

    
  
Comment on attachment 113763 [details] [diff] [review]
Treat <script for='window' event='onload()'> as inline script.

>Index: content/html/content/src/nsHTMLScriptElement.cpp
>===================================================================

>@@ -444,6 +445,62 @@
> NS_HTML_CONTENT_INTERFACE_MAP_END
> 
> 
>+// Helper method for checking if the script element has a "for"
>+// attribute with a value that matches "\s*window\s*", and an "event"
>+// attrivute that matches "\s*onload([ \(].*)?", both case

attribute

>+// insensitive. This is how IE seems to filter out a window's onload
>+// handler from a <script for=... event=...> element.
>+
>+PRBool
>+nsHTMLScriptElement::IsOnloadEventForWindow()
>+{
>+  nsAutoString str;
>+  nsresult rv = GetAttr(kNameSpaceID_None, nsHTMLAtoms::_for, str);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  str.CompressWhitespace();

I'd prefer if you searched for the first non-whitespace char, compared the next
6 chars to "window" and then the rest for whitespace chars. Or you could create
a utility function that returns the non-whitespace substring of a given string
with a start and an end iterator. I bet there's other places in content where
we could use that too :-).

>+
>+  if (!str.Equals(NS_LITERAL_STRING("window"),
>+                  nsCaseInsensitiveStringComparator())) {
>+    return PR_FALSE;
>+  }
>+
>+  // We foud for="window", now check for event="onload".

found

        Attachment #113763 -
        Flags: superreview?(peterv) → superreview+

    
    

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

    
    

    
  
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

    
    

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

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