link properties not reflected until user interacts with the link

VERIFIED FIXED in mozilla0.9

Status

()

Core
DOM: Core & HTML
P2
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: sean echevarria, Assigned: joki (gone))

Tracking

({dom0})

Trunk
mozilla0.9
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-][PDTP2][rtm-])

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
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.
(Reporter)

Comment 1

17 years ago
Created attachment 12975 [details]
testcase
(Reporter)

Comment 2

17 years ago
nominating nsbeta3
Keywords: 4xp, nsbeta3
(Reporter)

Comment 3

17 years ago
cc tim (this breaks the beatnik music object wizard extensions)

Comment 4

17 years ago
Marking nsbeta3-.
Whiteboard: [nsbeta3-][egk]
(Reporter)

Comment 5

17 years ago
What does 'egk' in the whiteboard mean?
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.
Keywords: beatnik
(Reporter)

Comment 7

17 years ago
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).
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?
Priority: P3 → P2
(Assignee)

Comment 9

17 years ago
Okay, I'll take this one.  We know the problem and have a plan for a fix.
Assignee: jst → joki
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Keywords: rtm
Whiteboard: [nsbeta3-][egk] → [egk]
*** Bug 46868 has been marked as a duplicate of this bug. ***

Comment 11

17 years ago
Marking nsbeta3+.  Tom knows how to fix this.
Keywords: rtm
Whiteboard: [egk] → [nsbeta3+]

Comment 12

17 years ago
PDT agrees P2
Whiteboard: [nsbeta3+] → [nsbeta3+][PDTP2]

Comment 13

17 years ago
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+
Keywords: rtm
Whiteboard: [nsbeta3+][PDTP2] → [nsbeta3-][PDTP2][rtm+]

Comment 14

17 years ago
PDT marking [rtm need info] until patch and code reviews are available.
Whiteboard: [nsbeta3-][PDTP2][rtm+] → [nsbeta3-][PDTP2][rtm need info]
(Assignee)

Comment 15

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

Comment 16

17 years ago
Created attachment 16831 [details] [diff] [review]
Proposed fix

Comment 17

17 years ago
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-.
Whiteboard: [nsbeta3-][PDTP2][rtm need info] → [nsbeta3-][PDTP2][rtm-]
(Reporter)

Comment 18

17 years ago
adding ns601 keyword
Keywords: ns601
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
Keywords: nsbeta3, rtm → mozilla0.9
Keywords: ns601
*** Bug 35463 has been marked as a duplicate of this bug. ***
*** Bug 37584 has been marked as a duplicate of this bug. ***
Nom. nsbeta1. DOM correctness bug that's also actually affecting a real web site
and plug-in (Beatnik).
Keywords: nsbeta1
(Assignee)

Comment 23

17 years ago
I still have this fix from the last rtm cycle.  I'll get it updated and checked 
in.
Target Milestone: --- → mozilla0.8
(Assignee)

Comment 24

17 years ago
*** Bug 57367 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 25

17 years ago
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.
Target Milestone: mozilla0.8 → mozilla0.9
Keywords: dom0
(Assignee)

Comment 26

16 years ago
Created attachment 27566 [details] [diff] [review]
Patch for 49120 and 53640, part 1
(Assignee)

Comment 27

16 years ago
Created attachment 27569 [details] [diff] [review]
Patch for 49120 and 53640, part 2
(Assignee)

Comment 28

16 years ago
sr = jst
(Assignee)

Comment 29

16 years ago
r = peterv

Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 30

16 years ago
vrfy
Status: RESOLVED → VERIFIED

Updated

16 years ago
Keywords: beatnik
You need to log in before you can comment on or make changes to this bug.