26402 - event handlers not compiled via element.setAttribute()

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

[kinmoz]

In my 02/03/2000 WinNt debug build. Selecting a previously viewed page under the
Browser's Go menu does nothing. It should take me to that page.

Comment 1

[kinmoz]

Cc [e-mail] since I'm not sure if radha is on sabbatical or not.

Comment 2

[Claudius Gayle]

yup. this is XP. note 'Back' and 'Forward' under 'Go' work fine, it's actually picking a site that is doing nothing. No console 
messages, nothing. This is also definitely a regression as it does not occur in hte 2000020108 WinNt builds. thanx kin.

Comment 3

[don]

Well, this ought to be simple to fix ...

Comment 4

[don]

Nominating as "beta1" blocker.

Comment 5

[leger]

Putting on PDT+ radar for beta1.

Comment 6

[Bill Law]

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

Comment 7

[Bill Law]

This might be a webshell/new-session-history thing.  Could be nasty.  I'll try 
to have a look ASAP to find out more.

Comment 8

[Christine Begle]

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

Comment 9

[Bill Law]

The menu items seem to be created correctly.  It seems their oncommand handlers 
are not getting fired.  I've heard reports that a similar bug has already been 
reported; I've pinged Chris Waterson to try to track that one down.

Will update as soon as I hear anything.

Comment 10

[Bill Law]

Dave Hyatt suggested a workaround.  I've tweaked nsBrowserInstance.cpp so that 
it now sets the oncommand handler attribute on the menu items *after* the 
element has been added to the document.  This bypasses a known problem relating 
to the timing of the compilation of such handlers.

I am reassigning this to Chris Waterson (it may be a dup of a bug he already 
has).  I've removed "beta1" from the keywords and would ask that the [PDT+] 
status now be removed.  The Go menu items now work so the root cause does not 
have to be fixed right now.

Comment 11

[Suresh Duddi (gone)]

Removing PDT+.

Waterson wont be in until monday to look at this.

Comment 12

[Chris Waterson]

*** This bug has been marked as a duplicate of 27050 ***

Comment 13

[Chris Waterson]

My bad. This is -not- a dup.

Comment 14

[Chris Waterson]

Taking bug. See also bug 27050 and 
news://news.mozilla.org/38A0E3AD.EB7882DD%40mozilla.org, where I 
incorrectly posted a bunch of junk.

I need to talk with vidur/joki to get a clear understanding of how the DOM 
working group expects properties and attributes to interact in the case of 
event handlers.

Also, rename bug to be what it really means.

Comment 15

[Claudius Gayle]

this is not a history bug anymore right

Comment 16

[Chris Waterson]

Attachment: proposed fix

Comment 17

[Chris Waterson]

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

Comment 18

[Robert John Churchill]

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

Comment 19

[David Hyatt]

Let's get this bad boy checked in.  Have you made sure inline style works as 
well?

Comment 20

[David Hyatt]

What's holding this bug up?  waterson? waterson? :)

Comment 21

[Chris Waterson]

bonk. fix checked in. r=hyatt

Comment 22

[Stuart Parmenter]

inline styles don't work :(

Comment 23

[David Hyatt]

Neither do the event handlers.  Looking at the code, I couldn't find evidence 
that this bug had been fixed (maybe I'm blind).  I didn't see a walk in 
SetDocument.

Comment 24

[Chris Waterson]

Look in AddSubtreeToDocument(). It's firing allright, because it helped keep 
the tree closed yesterday :-).

Comment 25

[David Hyatt]

Is AddSubtreeToDocument not called from SetDocument?  THat would explain why 
this didn't fix XBL, since anonymous content isn't ever placed in using DOM 
calls (there's only a back pointer to the parent from the child anonymous 
content).

Comment 26

[Chris Waterson]

No, it's not. It's only called when a "noisy" append/insert happens. Can you 
not do that? I'd be a bit nervous about adding the call on SetDocument() 
because it gets fired on a lot of different paths, but maybe that's the right 
place to do it...

I'll tinker with it.

Comment 27

[Chris Waterson]

Ok, I remember why we can't do it on nsXULElement::SetDocument(). We want to 
force HTML elements to update themselves as well. I guess we could just punt on 
HTML elements for now and make this be vidur & jst's problem. XUL would work.

Comment 28

[Chris Waterson]

Attachment: better fix. move to nsXULElement::SetDocument()

Comment 29

[David Hyatt]

Let's fix it in XUL and then reassign the bug to jst, who can presumably make a 
fix in HTML at his leisure.

Comment 30

[Chris Waterson]

I've checked in the second attempt. Johnny, you can take this bug if you want, 
or open a new one for HTML elements.

Comment 31

[Jan Carpenter]

Verified
2000-07-13-08-M17 : Linux
2000-07-13-09-M17 : WinNT & Win98
2000-07-13-08-M17 : Mac

Comment 32

[Blake Ross]

What are the odds that this is still broken for key events?  oncommand's set 
manually for tree and outliner columnpickers don't fire upon pressing enter, 
but they do when clicked.  (See 
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/o
utliner.xml#577)

That seems unlikely, though, so this is probably a separate bug.