Closed Bug 292953 Opened 19 years ago Closed 19 years ago

fastback/bfcache makes some event handlers inoperative?

Categories

(Core :: DOM: Events, defect)

Product:
Core
Component:
DOM: Events
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bryner)

References

Details

Attachments

(3 files)

Testcase
186 bytes, text/html
Details
IE onload enumeration test
257 bytes, text/html
Details
proposed change to JS_CompileUCFunctionForPrincipals
891 bytes, patch
shaver
: review+
Details | Diff | Splinter Review 
STEPS TO REPRODUCE:

1)  Get a build with bfcache
2)  Load attached testcase
3)  Follow directions.

EXPECTED RESULTS:  onresize handler fires on window resize

ACTUAL RESULTS: it doesn't
Attached file Testcase — 

    
  
Blocks: blazinglyfastback

    
    

    
  
Note that resize event WILL fire if you refresh the testcase, go back and then
go forward again, as long as you only do it once.  If you go back and forward at
least once more, however, the resize event will not fire.  Is the
PresShell-saving code not fired on refresh, or is there something that it's
doing that back and forward should but aren't?

The keypress handler also doesn't work with bfcache enabled, incidentally.

    
    

    
  
> Is the PresShell-saving code not fired on refresh

Right.

    
    

    
  
I think the problem here is that <body> handlers such as this one are installed
on the window object as nonenumerable properties.  So, while the event handler
is registered with the EventListenerManager, we fail to save or restore the JS
function object associated with it.

I think we can solve this in one of two ways:

1. Make the event handler properties enumerable
2. Add a mechanism to EventListenerManager to explicitly enumerate the
properties associated with registered event listeners.

I'd prefer option 1, since it's simpler.  Does anyone know what the reasoning is
for making these properties nonenumerable?


    
    

    
  
(In reply to comment #4)
> 1. Make the event handler properties enumerable
> 2. Add a mechanism to EventListenerManager to explicitly enumerate the
> properties associated with registered event listeners.
> 
> I'd prefer option 1, since it's simpler.  Does anyone know what the reasoning is
> for making these properties nonenumerable?

They've always been non-enumerable, which is probably just an old inconsistency
(i.e., a bug) from Netscape 2.  What does IE do?  If we can make 'em enumerable
without breaking any pages, great.

JS_CompileUCFunctionForPrincipals passes 0 for attrs to OBJ_DEFINE_PROPERTY, and
this is a frozen API.  Sucky.  For consistency with scripted functions, it would
be great to just change this API's implementation to pass JSPROP_ENUMERATE to
OBJ_DEFINE_PROPERTY.  I could make a new API, just to carry the attrs parameter.
We should make an attempt to discover or think of cases where content breaks if
we make this incompatible change.

/be

    
    

    
  
Inner window objects in 1.9 will avoid all this hassle, but I would love to just
fix this with a big fat JSPROP_ENUMERATE incompatible change.  Someone who can
test IE, try this:

<html>
<head>
<script>
function onload() {
  for (var i in this) {
    if (i == "onresize")
      alert(this[i]);
  }
}
</script>
</head>
<body onresize="alert('resized!')">
Whatever
</body>
</html>

    
    

    
  


  
This is a modified version of brendan's testcase. In IE the onload isn't run
based on the head namespace, you have to explicitly add it to the body tag.
Also added an alert to prove the onload has run.

Result: IE can enumerate the onload handler

    
    

    
  
Comment on attachment 186570 [details] [diff] [review]
proposed change to JS_CompileUCFunctionForPrincipals

We want _all_ such functions to be enumerable?	Is that better than passing a
flags parameter in?

    
    

    
  
Comment on attachment 186570 [details] [diff] [review]
proposed change to JS_CompileUCFunctionForPrincipals

Brendan convinced me that having the default be the same as for script-declared
functions is what we want.  Callers (who might be slightly surprised by this
change, but probably won't notice) can use JS_SetPropertyAttrs if they want to
do something different. r=shaver

        Attachment #186570 -
        Flags: review?(shaver) → review+

    
    

    
  
(In reply to comment #7)
> Created an attachment (id=186550) [edit]
> IE onload enumeration test
> 
> This is a modified version of brendan's testcase. In IE the onload isn't run
> based on the head namespace,

Dan, do you know whether there's a "head namespace"?  I thought IE had some kind
of inner window object they equate with the outer one (window, the persistent
reference returned by window.open), and they use these two to separate onload
from onload, if you get what I mean.

> you have to explicitly add it to the body tag.

Rather, call it from <body onload="...">.

> Also added an alert to prove the onload has run.
> 
> Result: IE can enumerate the onload handler

And it calls onresize multiple times during resizing -- nice.  Oh, and the
onresize handler converts to a string that looks like "function anonymous()
{...}" -- double-nice.

What a world.  We should not emulate any of this, except for enumerability.

/be
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED

    
    

    
  
Tested with this bug's first attachment, works now with the patch I just checked
in.  Someone else can mark VERIFIED.

/be

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: