656769 - Can't set duplicate bug number with IE9 except in compatibility view

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[dougbugzilla]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.17) Gecko/20110420 Firefox/3.6.17 ( .NET CLR 3.5.30729; .NET4.0E)
Build Identifier: 4.0

When the "Mark as duplicate" link is clicked, the bug status is changed to resolved, but the "DUPLICATE of _____" part doesn't show up.

Reproducible: Always

Comment 1

[Frédéric Buclin]

wow, you are right. I can reproduce the issue. :(

Comment 2

[dougbugzilla]

Attachment: after clicking Mark as duplicate in ie9

This is what ie9 shows. There are no script errors listed.

Comment 3

[dougbugzilla]

Attachment: This is what ie8 shows

This is what ie8 shows after clicking "Mark as duplicate"

Comment 4

[Frédéric Buclin]

For the record, Bugzilla 3.6.5 and 3.4.11 have the same issue. So we are doing something wrong with IE9.

Comment 5

[:glob ✱]

at this stage it looks like an issue with yui's event handling for ie9.

in compatibility mode, stepping into the fireEvent line in bz_fireEvent:
  return anElement.fireEvent('on' + anEvent, evt);
puts us into yui code.

in standard mode, stepping into that line doesn't do anything.


i've found a yui3 bug which sounds right, but nothing yet for yui2.
http://yuilibrary.com/projects/yui3/ticket/2529432
this at least indicates there are event changes introduced by ie9 which may impact yui2.

Comment 6

[:glob ✱]

Attachment: patch v1

this works around the issue by asking IE9 to work in IE8 mode.

Comment 7

[:glob ✱]

reported upstream in http://yuilibrary.com/projects/yui2/ticket/2529442

Comment 8

[:glob ✱]

Comment on attachment 532126 [details] [diff] [review]
patch v1

yui said:

You're referring to a DOM method that was changed to match W3C spec in version 9. Use the event-simulate or event-node-simulate module to call whichever method is appropriate for the given browser. E.g.

YUI().use('event-node-simulate', function (Y) {
    Y.one("#foo").on('click', function () { Y.log("I've been clicked!"); });
    setTimeout(function () { Y.one("#foo").simulate("click"); }, 2000);
});

i'll look at rewriting bz_fireEvent

Comment 9

[:glob ✱]

Attachment: patch v2

the sample code provided by yahoo is for yui3, so it can't be directly used.

yui3's event.simulate code categorises each event and uses different mechanisms for the different types of events (ui, mouse, key, etc).  a matching function doesn't exist in yui2.

as we only need to support ui events (currently the only event we trigger is 'change'), i've limited it to ui events, and renamed the function to bz_fireUIEvent.

the crux of the fix is we need to check for the dom compliant method first, before checking for ie.  ie9 supports doc.createEvent, so it will be used there.  older ie versions will continue to use the old method.

tested on ie9 and ie8.

Comment 10

[Max Kanat-Alexander]

Comment on attachment 533194 [details] [diff] [review]
patch v2

Review of attachment 533194 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/util.js
@@ +258,5 @@
> +    if (!uiEvents[anEvent]) {
> +        if (console)
> +            console.error(anEvent + ' is not a UI event, unable to fire with bz_fireUIEvent');
> +        return false;
> +    }

I think for now we should just have this in the docs, no need to specifically list out UI events here. In the future we'll be using YUI 3 or there will be something in YUI 2 that we can use.

@@ +260,5 @@
> +            console.error(anEvent + ' is not a UI event, unable to fire with bz_fireUIEvent');
> +        return false;
> +    }
> +
> +    if (YAHOO.lang.isFunction(document.createEvent)) {

You can't just do "if(document.createEvent)"?

@@ +262,5 @@
> +    }
> +
> +    if (YAHOO.lang.isFunction(document.createEvent)) {
> +        // DOM-compliant browser
> +        evt = document.createEvent("UIEvents");

From reading the DOM Spec, I think we're firing an HTMLEvent, not a UIEvent:

  http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-eventgroupings-htmlevents

  Perhaps this also means that our function should be called bz_fireHTMLEvent (or simply stay as bz_fireEvent, since it looks like all of our currently-used event types are covered as HTMLEvents).

@@ +263,5 @@
> +
> +    if (YAHOO.lang.isFunction(document.createEvent)) {
> +        // DOM-compliant browser
> +        evt = document.createEvent("UIEvents");
> +        evt.initUIEvent(anEvent, false, false, window, 1);

Are we sure that "false, false" is the normal default for onchange events? It looks like before, we were setting both to true.

@@ +266,5 @@
> +        evt = document.createEvent("UIEvents");
> +        evt.initUIEvent(anEvent, false, false, window, 1);
> +        return !anElement.dispatchEvent(evt);
> +
> +    } else if (YAHOO.lang.isObject(document.createEventObject)) {

This one I think we can just check if on, too, and no need to do the YAHOO.lang thing.

@@ +277,3 @@
>          return anElement.fireEvent('on' + anEvent, evt);
>      }
> +    return false;

Actually, we should never reach this point, so perhaps instead, we should just not have an "else if" block above?

Comment 11

[Max Kanat-Alexander]

In fact, it looks like all that may be needed in order to fix this bug is to reverse the order of checks in bz_fireEvent.

Comment 12

[:glob ✱]

Comment on attachment 533194 [details] [diff] [review]
patch v2

(In reply to comment #10)
> Comment on attachment 533194 [details] [diff] [review] [review]
> patch v2
> 
> You can't just do "if(document.createEvent)"?
> 
> Are we sure that "false, false" is the normal default for onchange events?
> It looks like before, we were setting both to true.
> 
> This one I think we can just check if on, too, and no need to do the
> YAHOO.lang thing.
> 
> Actually, we should never reach this point, so perhaps instead, we should
> just not have an "else if" block above?

sorry, i should have mentioned the code is a back-port of the relevant event.simulate code from yui3.

http://developer.yahoo.com/yui/3/api/event-simulate.js.html

> From reading the DOM Spec, I think we're firing an HTMLEvent, not a UIEvent:

i agree, but i think we're better off aiming for yui3 parity here.

> In fact, it looks like all that may be needed in order to fix this bug is to
> reverse the order of checks in bz_fireEvent.

the main reason for the back-port was yui3 fires a UIEvent; i have to assume this was done for a reason.

Comment 13

[Max Kanat-Alexander]

(In reply to comment #12)
> sorry, i should have mentioned the code is a back-port of the relevant
> event.simulate code from yui3.

  Ah. Since this is a branch fix, I'd rather just go with the smallest change. No need to backport the YUI 3 code.

> http://developer.yahoo.com/yui/3/api/event-simulate.js.html
> 
> > From reading the DOM Spec, I think we're firing an HTMLEvent, not a UIEvent:
> 
> i agree, but i think we're better off aiming for yui3 parity here.

  Mmm, I think we're better off aiming for DOM spec compliance. Also, perhaps it's event-node-simulate you should be copying, and that's why you see YUI 3 using the "wrong" event type.

Comment 14

[:glob ✱]

(In reply to comment #13)
> > i agree, but i think we're better off aiming for yui3 parity here.
> Mmm, I think we're better off aiming for DOM spec compliance.

i've raised this issue with yui, as this looks like a legitimate bug in yui3.

> Also, perhaps it's event-node-simulate you should be copying, and that's why
> you see YUI 3 using the "wrong" event type.

event-node-simulate doesn't exist, it's node-event-simulate.
http://developer.yahoo.com/yui/3/api/node-event-simulate.js.html

>   Ah. Since this is a branch fix, I'd rather just go with the smallest
> change. No need to backport the YUI 3 code.

no worries, i'll submit an updated patch with the minimal change.

Comment 15

[:glob ✱]

Attachment: patch v3

Comment 16

[Max Kanat-Alexander]

Comment on attachment 536284 [details] [diff] [review]
patch v3

Review of attachment 536284 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/util.js
@@ +253,4 @@
>          var evt = document.createEventObject();
>          return anElement.fireEvent('on' + anEvent, evt);
> +    } else {
> +        // broken browser

I don't think we support broken browsers. I'm happy to have the code die here, and just have an "else" above instead of an "else if". r+ with this clause removed and the above "else if" changed to just an "else".

Comment 17

[:glob ✱]

(In reply to comment #14)
> i've raised this issue with yui, as this looks like a legitimate bug in yui3.

http://yuilibrary.com/projects/yui3/ticket/2530379



Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified js/util.js
Committed revision 7840.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified js/util.js
Committed revision 7605.