fastback/bfcache makes some event handlers inoperative?

RESOLVED FIXED

Status

()

Core
DOM: Events
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: bz, Assigned: Brian Ryner (not reading))

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

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

Comment 2

13 years ago
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.
(Assignee)

Comment 4

13 years ago
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>
Created attachment 186550 [details]
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, 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
Created attachment 186570 [details] [diff] [review]
proposed change to JS_CompileUCFunctionForPrincipals
Attachment #186570 - Flags: review?(shaver)
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
Last Resolved: 13 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.