Last Comment Bug 330494 - Remove NS_EVENT_FLAG_INIT
: Remove NS_EVENT_FLAG_INIT
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
: Hixie (not reading bugmail)
Mentors:
: 9544 (view as bug list)
Depends on: 336584 234455 330710 358091
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-14 12:58 PST by Olli Pettay [:smaug]
Modified: 2007-11-02 02:51 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed patch (13.32 KB, patch)
2006-03-15 10:49 PST, Olli Pettay [:smaug]
no flags Details | Diff | Review
proposed patch (37.67 KB, patch)
2006-03-18 12:09 PST, Olli Pettay [:smaug]
no flags Details | Diff | Review
changed preventCapture/Bubble to no-op (37.16 KB, patch)
2006-03-26 09:45 PST, Olli Pettay [:smaug]
jonas: review+
jst: superreview+
Details | Diff | Review
to be checked in (37.16 KB, patch)
2006-04-10 09:57 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Review
Warnings for 1.8 (13.34 KB, patch)
2006-04-10 13:57 PDT, Olli Pettay [:smaug]
bzbarsky: approval‑branch‑1.8.1+
Details | Diff | Review

Description Olli Pettay [:smaug] 2006-03-14 12:58:40 PST
NS_EVENT_FLAG_INIT shouldn't be needed anymore.
In most cases it is trivial to remove, but there are two cases which I have to think about a bit.
http://lxr.mozilla.org/seamonkey/source/content/events/src/nsEventListenerManager.cpp#1617
http://lxr.mozilla.org/seamonkey/source/content/events/src/nsEventListenerManager.cpp#1623
In those cases it is easy to bring back or keep the old behaviour, but I want
to ensure that that is the right behaviour.
Comment 1 Olli Pettay [:smaug] 2006-03-14 13:10:55 PST
The fun part is ofc that those two cases are part of the NN4 style
event handling (which I really would like to get rid of).
Comment 2 Olli Pettay [:smaug] 2006-03-15 10:49:46 PST
Created attachment 215151 [details] [diff] [review]
proposed patch

This needs some explanation. And I'd like to get some comments also from jst.
This is after all touching some code which has been there since 1999
Comment 3 Olli Pettay [:smaug] 2006-03-15 11:14:20 PST
Comment on attachment 215151 [details] [diff] [review]
proposed patch


> NS_IMETHODIMP
> nsDOMEvent::PreventBubble()
> {
>-  if (mEvent->flags & NS_EVENT_FLAG_BUBBLE || mEvent->flags & NS_EVENT_FLAG_INIT) {
>-    mEvent->flags |= NS_EVENT_FLAG_STOP_DISPATCH;
>-  }
>+  // Warning but still allowing to use PreventBubble in capture phase,
>+  // because this method has worked in all phases for years :(
>+  NS_WARN_IF_FALSE((mEvent->flags & NS_EVENT_FLAG_BUBBLE) ||
>+                   !(mEvent->flags & NS_EVENT_FLAG_CAPTURE),
>+                   "Using nsIDOMEvent::PreventBubble in capture phase!");
>+  mEvent->flags |= NS_EVENT_FLAG_STOP_DISPATCH;
>   return NS_OK;
> }

NS_EVENT_FLAG_INIT was earlier (in the old event dispatching code) always set in the 
nsEvent.flags when the event was dispatched. It was cleared only from the aFlags 
parameter of the HandleDOMEvent, not from the nsEvent.flags.


> NS_IMETHODIMP
> nsDOMUIEvent::GetCancelBubble(PRBool* aCancelBubble)
> {
>   NS_ENSURE_ARG_POINTER(aCancelBubble);
>-  if (mEvent->flags & NS_EVENT_FLAG_BUBBLE || mEvent->flags & NS_EVENT_FLAG_INIT) {
>-    *aCancelBubble = (mEvent->flags &= NS_EVENT_FLAG_STOP_DISPATCH) ? PR_TRUE : 
> PR_FALSE;
>-  }

Same thing here. As far as I see mEvent->flags & NS_EVENT_FLAG_INIT means that
after dispatching the event the expression in |if| was always true in the old event 
dispathing code. 
Note, (mEvent->flags &= NS_EVENT_FLAG_STOP_DISPATCH) doesn't make sense to me, 
(mEvent->flags & NS_EVENT_FLAG_STOP_DISPATCH) does.


> NS_IMETHODIMP
> nsDOMUIEvent::SetCancelBubble(PRBool aCancelBubble)
> {
>-  if (mEvent->flags & NS_EVENT_FLAG_BUBBLE || mEvent->flags & NS_EVENT_FLAG_INIT) {

Same thing here.


>+++ content/events/src/nsEventDispatcher.cpp	15 Mar 2006 18:22:46 -0000
...
>   // Bubble
>+  aVisitor.mEvent->flags &= ~NS_EVENT_FLAG_CAPTURE;
>   if (!(aVisitor.mEvent->flags & NS_EVENT_FLAG_CANT_BUBBLE)) {
>-    aVisitor.mEvent->flags &= ~NS_EVENT_FLAG_CAPTURE;

This is just because I want to ensure that after dispatching the event, 
the flags related to phases are removed from the events.


>+++ content/events/src/nsEventListenerManager.cpp	15 Mar 2006 18:22:48 -0000
>@@ -1606,25 +1606,25 @@ nsEventListenerManager::HandleEventSubTy
>                                            PRUint32 aSubType,
>                                            PRUint32 aPhaseFlags)
> {
>   nsresult result = NS_OK;
> 
>   // If this is a script handler and we haven't yet
>   // compiled the event handler itself
>   if (aListenerStruct->mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) {
>-    // If we're not in the capture phase we must *NOT* have capture flags
>-    // set.  Compiled script handlers are one or the other, not both.
>-    if (aPhaseFlags & NS_EVENT_FLAG_BUBBLE && !aPhaseFlags & NS_EVENT_FLAG_INIT) {
>+    // If we're not in the capture phase we must have bubble flag set.
>+    if ((aPhaseFlags & NS_EVENT_FLAG_BUBBLE)) {

this is my best guess, really. !aPhaseFlags & NS_EVENT_FLAG_INIT is just so bizarre.
Has it ever done anything useful.


>       if (aListenerStruct->mSubTypeCapture & aSubType) {
>         return result;
>       }
>     }
>     // If we're in the capture phase we must have capture flags set.
>-    else if (aPhaseFlags & NS_EVENT_FLAG_CAPTURE && !aPhaseFlags & NS_EVENT_FLAG_INIT) {
>+    else if ((aPhaseFlags & NS_EVENT_FLAG_CAPTURE) &&
>+             !(aPhaseFlags & NS_EVENT_FLAG_BUBBLE)) {

Same here. !aPhaseFlags & NS_EVENT_FLAG_INIT ??? That should be !(aPhaseFlags & NS_EVENT_FLAG_INIT) I think.
This is anyway code for the broken NN4 event handling, which should be removed IMO.
With normal (script) event listeners the first case (bubbling) is used and 
(aListenerStruct->mSubTypeCapture & aSubType) evaluates to false, so that code isn't really used. At least if I read the code right ;)



>@@ -1669,19 +1669,16 @@ nsEventListenerManager::HandleEvent(nsPr
> {
>   NS_ENSURE_ARG_POINTER(aEventStatus);
>   nsresult ret = NS_OK;
> 
>   if (aEvent->flags & NS_EVENT_FLAG_STOP_DISPATCH) {
>     return ret;
>   }
> 
>-  if (aFlags & NS_EVENT_FLAG_INIT) {
>-    aFlags |= (NS_EVENT_FLAG_BUBBLE | NS_EVENT_FLAG_CAPTURE);
>-  }

Event phase flags are set already in nsEventDispatcher


>+++ widget/public/nsGUIEvent.h	15 Mar 2006 18:23:33 -0000
>@@ -99,40 +99,33 @@ class nsIDOMEventTarget;
> #define NS_SVG_EVENT                      30
> #define NS_SVGZOOM_EVENT                  31
> #endif // MOZ_SVG
> 
> // These flags are sort of a mess. They're sort of shared between event
> // listener flags and event flags, but only some of them. You've been
> // warned!
> #define NS_EVENT_FLAG_NONE                0x0000
>-#define NS_EVENT_FLAG_INIT                0x0001
>+#define NS_EVENT_FLAG_TRUSTED             0x0001

Removing NS_EVENT_FLAG_INIT so 0x0001 can be used now for NS_EVENT_FLAG_TRUSTED, 
and this means that PRUint32 internalAppFlags is not needed anymore in nsEvent.
Comment 4 Olli Pettay [:smaug] 2006-03-15 11:18:54 PST
(In reply to comment #3)
> 
> Same thing here. As far as I see mEvent->flags & NS_EVENT_FLAG_INIT means that
> after dispatching the event the expression in |if| was always true
Er, not after, but while dispathing...

Comment 5 Olli Pettay [:smaug] 2006-03-15 12:42:36 PST
I also tested NN4 event handling (using some test cases in bugzilla) and after
this patch it is just as broken as before.
Comment 6 Boris Zbarsky [:bz] 2006-03-15 13:49:22 PST
So preventBubble is not defined by any specs, right?  Why are we even using it?  Could we fix up our tree to not use it (and use stopPropagation instead) and then nuke it?  Or would that break extensions?

I'll try to get to this today, but if I don't it'll be a few days before I can look.
Comment 7 Alex Vincent [:WeirdAl] 2006-03-15 13:51:50 PST
Re preventBubble: see bug 105280
Comment 8 Olli Pettay [:smaug] 2006-03-15 14:12:20 PST
(In reply to comment #6)
> So preventBubble is not defined by any specs, right?  Why are we even using it?
>  Could we fix up our tree to not use it (and use stopPropagation instead) and
> then nuke it?  Or would that break extensions?

I'm afraid it could break some extensions. I even remember that some
extension tutorial mentioned preventBubble, not stopPropagation :(
But we should still fix our tree to not use it.

What I'm more interested to remove is the NN4 event handling code, which has
been broken forever. Maybe leaving empty methods to nsGlobalObject and nsDocument which just throw NS_ERROR_DOM_NOT_SUPPORTED_ERR or something like that.

Comment 9 Jonas Sicking (:sicking) 2006-03-15 16:47:41 PST
Why not kill both :)

Right now is a good time to remove preventBubble, if we do it only on trunk extension authors will have plenty of time to discover and fix their code. We could even make the function send some useful message to the js-console in 1.9 and remove it compleately after that.

For the NN4 methods I'd propose we make the no-ops rather then throwing. That will have a much smaller chance of breaking anyone.
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2006-03-15 22:21:09 PST
(In reply to comment #9)
> For the NN4 methods I'd propose we make the no-ops rather then throwing. That
> will have a much smaller chance of breaking anyone.

Indeed, exceptions or lack of functions stop executions, no-ops don't.
Comment 11 Olli Pettay [:smaug] 2006-03-16 10:07:03 PST
(In reply to comment #9)
> Why not kill both :)

Yes! 

> 
> Right now is a good time to remove preventBubble, if we do it only on trunk
> extension authors will have plenty of time to discover and fix their code. We
> could even make the function send some useful message to the js-console in 1.9
> and remove it compleately after that.

Sounds good.

> 
> For the NN4 methods I'd propose we make the no-ops rather then throwing. That
> will have a much smaller chance of breaking anyone.
>

True.

Patch coming. If not today then tomorrow. 
So I'll remove NN4 event handling but leave empty methods and 
I'll make preventBubble and preventCapture to print something
to JS console. And open a bug to replace preventCapture/preventBubble with
stopPropagation in the tree.

Comment 12 Jonas Sicking (:sicking) 2006-03-16 10:44:23 PST
Please put the warning in the NN4 methods too (at least the ones that aren't already no-ops).
Comment 13 Mike Connor [:mconnor] 2006-03-16 11:26:51 PST
rather than a debug build warning, can we dump to the js console?

Also, in order to help authors migrate, can we make preventBubble dump a deprecated usage warning to the js console on the 1.8 branch?
Comment 14 Jonas Sicking (:sicking) 2006-03-16 12:30:57 PST
Yes, that is what i was suggesting
Comment 15 Jonas Sicking (:sicking) 2006-03-16 12:31:55 PST
Oh, adding the warnings (but leaving the functionality) on the 1.8 branch is an excellent idea.
Comment 16 Olli Pettay [:smaug] 2006-03-18 12:09:50 PST
Created attachment 215520 [details] [diff] [review]
proposed patch

This removes the old (and broken) NN4.x event handling, adds
JS Console warnings and removes NS_EVENT_FLAG_INIT.
This patch is for trunk. Bug 330710 needs to be checked in before this, 
otherwise we'll see warnings coming also from chrome.

Sicking perhaps you have time the check this?
Comment 17 neil@parkwaycc.co.uk 2006-03-18 12:50:57 PST
Comment on attachment 215520 [details] [diff] [review]
proposed patch

>+static void
>+ReportUseOfDeprecatedMethod(nsGlobalWindow* aWindow, const char* aWarning)
>+{
>+  nsCOMPtr<nsIDOMDocument> domDoc;
>+  aWindow->GetDocument(getter_AddRefs(domDoc));
>+  nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
Am I missing something or is this not the same as aWindow->mDoc (also IMHO instead of passing "this" each time you should change it from a static to a member function).
Comment 18 Olli Pettay [:smaug] 2006-03-18 18:33:09 PST
(In reply to comment #17)
> (From update of attachment 215520 [details] [diff] [review] [edit])
> >+static void
> >+ReportUseOfDeprecatedMethod(nsGlobalWindow* aWindow, const char* aWarning)
> >+{
> >+  nsCOMPtr<nsIDOMDocument> domDoc;
> >+  aWindow->GetDocument(getter_AddRefs(domDoc));
> >+  nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
> Am I missing something or is this not the same as aWindow->mDoc (also IMHO
> instead of passing "this" each time you should change it from a static to a
> member function).
> 

It is not the same. GetDocument handles the outer/inner window thing.
(and IMO, in this case there is no need the have the function as a member method)
Comment 19 Jonas Sicking (:sicking) 2006-03-18 19:55:33 PST
Comment on attachment 215520 [details] [diff] [review]
proposed patch

>Index: content/events/src/nsDOMEvent.cpp
>@@ -253,19 +254,19 @@ nsDOMEvent::HasOriginalTarget(PRBool* aR
>   *aResult = !!(mEvent->originalTarget);
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
> nsDOMEvent::SetTrusted(PRBool aTrusted)
> {
>   if (aTrusted) {
>-    mEvent->internalAppFlags |= NS_APP_EVENT_FLAG_TRUSTED;
>+    mEvent->flags |= NS_EVENT_FLAG_TRUSTED;
>   } else {
>-    mEvent->internalAppFlags &= ~NS_APP_EVENT_FLAG_TRUSTED;
>+    mEvent->flags &= ~NS_EVENT_FLAG_TRUSTED;
>   }

Are these changes related to this?

> nsDOMEvent::PreventBubble()
> {
>-  if (mEvent->flags & NS_EVENT_FLAG_BUBBLE || mEvent->flags & NS_EVENT_FLAG_INIT) {
>-    mEvent->flags |= NS_EVENT_FLAG_STOP_DISPATCH;
>-  }
>+  ReportUseOfDeprecatedMethod(mEvent, this, "UseOfPreventBubbleWarning");
>+  mEvent->flags |= NS_EVENT_FLAG_STOP_DISPATCH;
>   return NS_OK;
> }

This won't leave the function is a no-op, will it?

> NS_IMETHODIMP
> nsDOMEvent::PreventCapture()
> {
>+
>+  ReportUseOfDeprecatedMethod(mEvent, this, "UseOfPreventCaptureWarning");
>   if (mEvent->flags & NS_EVENT_FLAG_CAPTURE) {
>     mEvent->flags |= NS_EVENT_FLAG_STOP_DISPATCH;
>   }
>   return NS_OK;
> }

Same here.

Still looking...
Comment 20 Olli Pettay [:smaug] 2006-03-19 02:53:49 PST
(In reply to comment #19)
> > {
> >   if (aTrusted) {
> >-    mEvent->internalAppFlags |= NS_APP_EVENT_FLAG_TRUSTED;
> >+    mEvent->flags |= NS_EVENT_FLAG_TRUSTED;
> >   } else {
> >-    mEvent->internalAppFlags &= ~NS_APP_EVENT_FLAG_TRUSTED;
> >+    mEvent->flags &= ~NS_EVENT_FLAG_TRUSTED;
> >   }
> 
> Are these changes related to this?

Yes, because removing NS_EVENT_FLAG_INIT there is one spare bit in nsEvent::flag and so nsEvent::internalAppFlags can be removed.


> 
> > nsDOMEvent::PreventBubble()
> > {
> >-  if (mEvent->flags & NS_EVENT_FLAG_BUBBLE || mEvent->flags & NS_EVENT_FLAG_INIT) {
> >-    mEvent->flags |= NS_EVENT_FLAG_STOP_DISPATCH;
> >-  }
> >+  ReportUseOfDeprecatedMethod(mEvent, this, "UseOfPreventBubbleWarning");
> >+  mEvent->flags |= NS_EVENT_FLAG_STOP_DISPATCH;
> >   return NS_OK;
> > }
> 
> This won't leave the function is a no-op, will it?

I guess I misunderstood your suggestion in #9. I thought you wanted only
NN4 methods to be really no-ops.
Comment 21 Olli Pettay [:smaug] 2006-03-26 09:45:11 PST
Created attachment 216327 [details] [diff] [review]
changed preventCapture/Bubble to no-op

trunk doesn't contain preventCapture/Bubble calls anymore,
so now those methods can be changed to no-ops (but with warning to JSConsole).
nsDOMUIEvent::Get/SetCancelBubble is however something different. IIRC
IE has a property called cancelBubble, so those methods can't be made
no-ops.
Comment 22 Jonas Sicking (:sicking) 2006-04-04 00:03:59 PDT
Comment on attachment 216327 [details] [diff] [review]
changed preventCapture/Bubble to no-op

>Index: content/html/document/src/nsHTMLDocument.cpp
>+static void
>+ReportUseOfDeprecatedMethod(nsHTMLDocument* aDoc, const char* aWarning)
>+{
>+  nsContentUtils::ReportToConsole(nsContentUtils::eDOM_PROPERTIES,
>+                                  aWarning,
>+                                  nsnull, 0,
>+                                  NS_STATIC_CAST(nsIDocument*, aDoc)->
>+                                    GetDocumentURI(),

This cast shouldn't be needed.


>Index: dom/locales/en-US/chrome/dom/dom.properties
>+UseOfCaptureEventsWarning=Use of captureEvents() is deprecated.
>+UseOfReleaseEventsWarning=Use of releaseEvents() is deprecated.
>+UseOfRouteEventWarning=Use of routeEvent() is deprecated.

Would be nice with a link to a wiki page, or to this bug.

>+UseOfPreventBubbleWarning=Event=%S, use of preventBubble() is deprecated. Use W3C standard stopPropagation() instead.
>+UseOfPreventCaptureWarning=Event=%S, use of preventCapture() is deprecated. Use W3C standard stopPropagation() instead.

Maybe nicer to say something like "preventBubble() called in handler for event %S. This method is deprecated, use..."

Up to you

r=me, though it'd be nice to have someone that knows the eventcode a bit better then me have a look at it as well.
Comment 23 Olli Pettay [:smaug] 2006-04-04 00:09:00 PDT
Comment on attachment 216327 [details] [diff] [review]
changed preventCapture/Bubble to no-op

jst should know event bits, so asking sr
Comment 24 Johnny Stenback (:jst, jst@mozilla.com) 2006-04-10 09:11:22 PDT
Comment on attachment 216327 [details] [diff] [review]
changed preventCapture/Bubble to no-op

In nsGlobalWindow.cpp, ReportUseOfDeprecatedMethod():

+  nsCOMPtr<nsIDOMDocument> domDoc;
+  aWindow->GetDocument(getter_AddRefs(domDoc));
+  nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);

This could simply be:

+  nsCOMPtr<nsIDocument> doc =
+    do_QueryInterface(aWindow->GetExtantDocument());

sr=jst
Comment 25 Olli Pettay [:smaug] 2006-04-10 09:57:51 PDT
Created attachment 217875 [details] [diff] [review]
to be checked in

The cast in nsHTMLDocument is needed.
Comment 26 jag (Peter Annema) 2006-04-10 12:26:07 PDT
/me wonders what the compiler's error message was.
Comment 27 Olli Pettay [:smaug] 2006-04-10 12:29:49 PDT
(In reply to comment #26)
> /me wonders what the compiler's error message was.
> 
Ambiguous something something

Comment 28 Olli Pettay [:smaug] 2006-04-10 13:57:58 PDT
Created attachment 217908 [details] [diff] [review]
Warnings for 1.8

this has the warnings from the trunk patch, but doesn't remove any functionality.
Comment 29 Mike Connor [:mconnor] 2006-04-10 22:40:32 PDT
jst or bz should be the one to ask for branch approval for this patch (I'm not a peer for DOM/content!)
Comment 30 Boris Zbarsky [:bz] 2006-04-11 03:54:04 PDT
Comment on attachment 217908 [details] [diff] [review]
Warnings for 1.8

This looks OK generally; I assume that with bug 330710 fixed on branch we won't get warnings from our own chrome there?

One thing I'd change is that I think the aCategory for all the ReportToConsole calls should be the same (I'd use "DOM Events").  That would make a lot more sense in the long run (eg if console ever gets decent category filtering, which I'm still hoping it will) than having an event-related warning in the "HTML Document" category.  This comment applies to the trunk patch as well.

Also, it might make sense to give the URI to the bug, not the bug number.  But either way.  ;)
Comment 31 Olli Pettay [:smaug] 2006-04-11 10:46:11 PDT
Checked in to branch and changed the catogory name (now using "DOM Events" in every case) in branch and trunk.
Comment 32 Gary Johnson 2006-09-08 15:12:17 PDT
What are we to do to capture keypress?

I only use events to detect hitting return on a multi form page.

example

function netscapeKeyPress(ev)
{
    if (ev.which == 13)
    {
        alert('enter does nothing here');
   }

}

function microsoftKeyPress()
{
    if (window.event.keyCode == 13)
   {
        alert('enter does nothing here');
   }

}

if (isGecko())
{
    window.captureEvents(Event.KEYPRESS);
    window.onKeyPress = netscapeKeyPress;

} 


Per the spec, their are no keypress, keyclick events
Comment 33 Johnny Stenback (:jst, jst@mozilla.com) 2006-09-08 15:19:36 PDT
(In reply to comment #32)
> if (isGecko())
> {
>     window.captureEvents(Event.KEYPRESS);
>     window.onKeyPress = netscapeKeyPress;
> 
> } 

For the record, window.captureEvents() is *not* supported by gecko. If you want to register a capture handler in gecko use the DOM standard addEventListener() and pass in the appropriate arguments to make your listener a capturing one.
Comment 34 Gérard Talbot 2006-09-30 13:41:56 PDT
Hello,

I wish we could reopen this bug and change the respective error messages

+UseOfCaptureEventsWarning=Use of captureEvents() is deprecated.
+UseOfReleaseEventsWarning=Use of releaseEvents() is deprecated.
+UseOfRouteEventWarning=Use of routeEvent() is deprecated.

to something like

UseOfCaptureEventsWarning=Use of captureEvents() is deprecated. To upgrade your code, use the DOM 2 addEventListener() method. For more help http://developer.mozilla.org/en/docs/DOM:element.addEventListener

UseOfReleaseEventsWarning=Use of releaseEvents() is deprecated. To upgrade your code, use the DOM 2 removeEventListener() method. For more help http://developer.mozilla.org/en/docs/DOM:element.removeEventListener

UseOfRouteEventWarning=Use of routeEvent() is deprecated. To upgrade your code, use the DOM 2 dispatchEvent() method. For more help
http://developer.mozilla.org/en/docs/DOM:element.dispatchEvent

The spirit behind the new messages is to be as helpful as possible.
Comment 35 Gérard Talbot 2006-09-30 13:57:56 PDT
Currently, the warning message in the error console will mention 

"Warning: Use of captureEvents() is deprecated, see bug 330494."

but this is not useful, not helpful nor ideally contextual. Improving the warning message along the lines of what I suggested would help resolving both bug 9544 and bug 132132 altogether.
Comment 36 Olli Pettay [:smaug] 2006-09-30 16:49:05 PDT
(In reply to comment #34)
> Hello,
> 
> I wish we could reopen this bug and change the respective error messages
> 

If you want better error messages, please file a new bug (and propose a patch ;) )
Comment 37 M.J.G. 2006-10-25 09:31:52 PDT
Unfortunately, this breaks the interface for "money transfers" with my bank (http://www.comdirect.de). Works with FF 1.5, doesn't with FF 2.0. FF 2.0 spews out the warning about captureEvents(), and the banking transaction aborts.

Is there anything I can do short of getting the bank change their code? Is there at least some info on how to replace the code that I can point them to? 

I can't believe this function got nooped without any transition period. We talk so much about backwards compatibility in other bugs, giving up standards compliance in turn. Why no grace period with warnings instead of breaking things?

Michael
Comment 38 Jonas Sicking (:sicking) 2006-10-25 09:44:13 PDT
No sites should break in FF2 due to the changes in this bug. The only changes to FF2 was to add warnings, all scripts should still work the same way as before. In other words, the grace period is now.
Comment 39 Olli Pettay [:smaug] 2006-10-25 11:53:32 PDT
(In reply to comment #37)
> Unfortunately, this breaks the interface for "money transfers" with my bank
> (http://www.comdirect.de). Works with FF 1.5, doesn't with FF 2.0. 

Any chance that you could try to find when the regression happened,
i.e. test some old nightly FF2 builds to see what was the last build with which the site works.

As Jonas said, there were only warning added to FF2, functionality didn't change. So this bug shouldn't have caused the regression.

Comment 40 M.J.G. 2006-10-26 02:05:46 PDT
(In reply to comment #38)
> No sites should break in FF2 due to the changes in this bug. The only changes
> to FF2 was to add warnings, all scripts should still work the same way as
> before. In other words, the grace period is now.
> 

(In reply to comment #39)
> (In reply to comment #37)
> > Unfortunately, this breaks the interface for "money transfers" with my bank
> > (http://www.comdirect.de). Works with FF 1.5, doesn't with FF 2.0. 
> 
> Any chance that you could try to find when the regression happened,
> i.e. test some old nightly FF2 builds to see what was the last build with which
> the site works.
> 
> As Jonas said, there were only warning added to FF2, functionality didn't
> change. So this bug shouldn't have caused the regression.
> 

Uh, I see now that I misunderstood the previous comments here: It's nooped for trunk, but on the 1.8.1 branch there are only warnings. I noticed things stopped working when I switched from 1.5 to 2.0, and the captureEvent() warnings where the only clue on the console; thus my suspicion about this bug.

Regarding the bank: I have to make an actual transfer attempt (EFT) each time; trying unsuccessfully with a number of nightlies should ring the bank's alarm bells. Trying successfully will use up my TANs. I'll see what I can do, though.

Michael
Comment 41 M.J.G. 2006-10-26 02:33:17 PDT
OK guys, sorry for ringing the bells unnecessarily:

I tried with another bank, and it didn't work either. Then, I tried from a fresh profile without any extensions: hooray, both banking sites work happily with FF 2.0. There are several warnings about captureEvents() and others, but things work anyways.

I guess it's time to set up a fresh profile, and/or install extensions one by one :(

Michael
Comment 42 Gérard Talbot 2007-02-02 22:41:10 PST
*** Bug 9544 has been marked as a duplicate of this bug. ***
Comment 43 Gérard Talbot 2007-02-02 22:45:58 PST
*** Bug 132132 has been marked as a duplicate of this bug. ***
Comment 44 Konstantin Svist 2007-02-27 12:59:23 PST
Please check the site hipcal.com, it seems to trigger an error message that mentions this bug #
Comment 45 Jonas Sicking (:sicking) 2007-02-27 13:17:04 PST
Please contact their site and ask them to fix their scripts and point them to

http://developer.mozilla.org/en/docs/Gecko_1.9_Changes_affecting_websites
Comment 46 Per Hansen 2007-11-01 04:36:35 PDT
(In reply to comment #34)
> Hello,
> 
> I wish we could reopen this bug and change the respective error messages
> 
<Snip>
I think this is an excellent idea, I just spend an hour finding this information and it would have been nice if it had just been in the error message.
Comment 47 Olli Pettay [:smaug] 2007-11-01 07:11:39 PDT
Please open a new bug (hopefully with a patch or at least suggestion for a better error message) and make it block this one.
Comment 48 Per Hansen 2007-11-02 02:51:29 PDT
I have now copied comment 34 to a new bug https://bugzilla.mozilla.org/show_bug.cgi?id=402181

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