Last Comment Bug 49120 - link properties not reflected until user interacts with the link
: link properties not reflected until user interacts with the link
Status: VERIFIED FIXED
[nsbeta3-][PDTP2][rtm-]
: dom0
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: x86 Windows 2000
: P2 normal (vote)
: mozilla0.9
Assigned To: joki (gone)
: Prashant Desale
: Andrew Overholt [:overholt]
Mentors:
: 35463 37584 46868 57367 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-08-15 16:14 PDT by sean echevarria
Modified: 2001-07-06 14:08 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.01 KB, text/html)
2000-08-15 16:15 PDT, sean echevarria
no flags Details
Proposed fix (26.91 KB, patch)
2000-10-11 17:41 PDT, joki (gone)
no flags Details | Diff | Splinter Review
Patch for 49120 and 53640, part 1 (57.78 KB, patch)
2001-03-13 02:52 PST, joki (gone)
no flags Details | Diff | Splinter Review
Patch for 49120 and 53640, part 2 (11.94 KB, patch)
2001-03-13 02:53 PST, joki (gone)
no flags Details | Diff | Splinter Review

Description sean echevarria 2000-08-15 16:14:48 PDT
Link properties like onmouseover are not accessible from javascript until the
user has interacted with the link.

They should be fully reflected by the time that a onload handler is called.
Comment 1 sean echevarria 2000-08-15 16:15:21 PDT
Created attachment 12975 [details]
testcase
Comment 2 sean echevarria 2000-08-15 16:17:38 PDT
nominating nsbeta3
Comment 3 sean echevarria 2000-08-16 19:29:44 PDT
cc tim (this breaks the beatnik music object wizard extensions)
Comment 4 Nisheeth Ranjan 2000-08-30 16:18:32 PDT
Marking nsbeta3-.
Comment 5 sean echevarria 2000-08-30 16:38:15 PDT
What does 'egk' in the whiteboard mean?
Comment 6 ekrock's old account (dead) 2000-09-01 15:24:36 PDT
It means it's a bug that I'm quite concerned about having nsbeta3-ed and am 
marking for possible status review or exception fix status for RTM.
Comment 7 sean echevarria 2000-09-01 17:08:26 PDT
This breaks link mouseovers on the Beatnik website.  The mouseovers are handled 
by a js script that we distribute to content developers - so problem is not 
limited to www.beatnik.com.

Rather than adding mouseover code by hand to every link, it is done dynamically 
via a sourced in js script to add sound effects, etc to links and form elements.

The documentation for the script is at:
http://www.beatnik.com/software/documentation/music_object/index.html
In the index frame, scroll down to the section labeled "Music Object 
Extensions" - the Wizards extension works with mouseover and mouseclick 
properties of links.

A more direct url to the documentation (less the frames):
http://www.beatnik.com/software/documentation/music_object/music-object-
reference/extensions/wizards/index.html

I previously wrote that the link properties should be fully reflected by the 
time that the document loads.  I think this can be refined: link properties 
should be fully reflected when they are accessed via js (even if the user has 
not interacted with them).
Comment 8 ekrock's old account (dead) 2000-09-14 17:16:29 PDT
Increasing priority to P2 as this is high profile backward compatibility
(breaking JS 1.0 DOM0 and showing up on beatnik.com site). No workaround either.
This would qualify for a check-in through 9/21 with mozilla reviewers approval.
Can we find someone to fix this?
Comment 9 joki (gone) 2000-09-14 19:29:40 PDT
Okay, I'll take this one.  We know the problem and have a plan for a fix.
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2000-09-15 14:27:08 PDT
*** Bug 46868 has been marked as a duplicate of this bug. ***
Comment 11 Nisheeth Ranjan 2000-09-18 17:22:53 PDT
Marking nsbeta3+.  Tom knows how to fix this.
Comment 12 Phil Peterson 2000-09-20 18:04:17 PDT
PDT agrees P2
Comment 13 Nisheeth Ranjan 2000-09-26 14:39:20 PDT
This didn't make the beta 3 train.  Marking nsbeta3- and nominating for rtm.
Requesting an exception fix approval from PDT by marking this rtm+
Comment 14 Phil Peterson 2000-10-02 18:35:02 PDT
PDT marking [rtm need info] until patch and code reviews are available.
Comment 15 joki (gone) 2000-10-11 17:40:46 PDT
Okay, I've got the fix ready and tested for this.  Basically what I did is to 
add an CompileScriptEventListener call onto nsIEventListenerManager.  I then 
called into this in the JS Resolve methods on nsDocument and nsGenericElement.  
I also added it into the Get{event handler name} methods on nsGlobalWindow.  So 
if someone asks for a JS event listener, we force it to compile, if it exists.  
Otherwise we do nothing.
Comment 16 joki (gone) 2000-10-11 17:41:18 PDT
Created attachment 16831 [details] [diff] [review]
Proposed fix
Comment 17 Nisheeth Ranjan 2000-10-12 23:22:33 PDT
Tom, please check this into the trunk after doing pre-checkin testing.  
Unfortunately, it'll be difficult to justify checking this fix into the rtm 
branch.

Marking rtm-.
Comment 18 sean echevarria 2000-10-26 18:05:45 PDT
adding ns601 keyword
Comment 19 Brendan Eich [:brendan] 2000-11-06 13:07:03 PST
I don't think ns601 means "please get in for Netscape 6.01", but maybe ekrock 
can say more about this misnamed keyword.

I removed nsbeta3 and rtm, and added mozilla0.9.  joki, is this on the trunk 
yet?  I don't see reviewer-stamps in the bug; I have a few comments myself:

+    nsAutoString mPropName, mPrefix;
+    mPropName.Assign(NS_REINTERPRET_CAST(const PRUnichar*, 
JS_GetStringChars(JS_ValueToString(aContext, aID))));
+    if (mPropName.Length() > 2)
+      mPrefix.Assign(mPropName.GetUnicode(), 2);
+    if (mPrefix.EqualsWithConversion("on")) {

and similar code in nsGenericElement.cpp should not copy into two nsAutoStrings 
just to check for "on" (also, this particular case should not use the mPropName 
convention reserved for class members, but that variable's unnecessary anyway).

Instead, use a zero-string-copy test:

+    JSString *str = JSVAL_TO_STRING(aID);
+    if (JS_GetStringLength(str) > 2) {
+      const jschar *chars = JS_GetStringChars(str)));
+      if (chars[0] == 'o' && chars[1] == 'n') {

Does the new "replaceable" attribute in IDL help at all in shrinking the size of 
this patch?  It can be used to define properties lazily from resolve. Cc'ing jst 
and vidur for advice.

/be
Comment 20 Johnny Stenback (:jst, jst@mozilla.com) 2001-01-17 22:49:00 PST
*** Bug 35463 has been marked as a duplicate of this bug. ***
Comment 21 Johnny Stenback (:jst, jst@mozilla.com) 2001-01-17 22:52:57 PST
*** Bug 37584 has been marked as a duplicate of this bug. ***
Comment 22 ekrock's old account (dead) 2001-01-18 13:35:50 PST
Nom. nsbeta1. DOM correctness bug that's also actually affecting a real web site
and plug-in (Beatnik).
Comment 23 joki (gone) 2001-01-22 12:36:54 PST
I still have this fix from the last rtm cycle.  I'll get it updated and checked 
in.
Comment 24 joki (gone) 2001-01-22 14:35:01 PST
*** Bug 57367 has been marked as a duplicate of this bug. ***
Comment 25 joki (gone) 2001-02-06 17:11:44 PST
I've been talking with jst as I integrated this patch and we're going to change 
the code this lives in in the next week or two.  So instead of putting this in 
now and then rewriting it next week I'm going to move this out to moz0.9 and put 
in the fix with the new code.
Comment 26 joki (gone) 2001-03-13 02:52:59 PST
Created attachment 27566 [details] [diff] [review]
Patch for 49120 and 53640, part 1
Comment 27 joki (gone) 2001-03-13 02:53:55 PST
Created attachment 27569 [details] [diff] [review]
Patch for 49120 and 53640, part 2
Comment 28 joki (gone) 2001-03-13 02:54:16 PST
sr = jst
Comment 29 joki (gone) 2001-03-13 05:01:59 PST
r = peterv

Fixed.
Comment 30 sean echevarria 2001-03-14 13:23:29 PST
vrfy

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