Bug 66752 – committers-autocomplete.js works only with WebKit based browsers

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

Committers automplete tool doesn't work with other browsers: Firefox, IE, Opera

[Csaba Osztrogonác]

Created attachment 104807 [details]
WIP patch

I played a little bit with it. This WIP patch made FF and 
Chrome happy, but unfortunately Opera is still sad because 
of a security problem. (Maybe because of XHR.) And IE is 
still sad, I don't know why.

[Peter Gal]

Created attachment 164558 [details]
proposed modification

Two things to notice here:
 1) using 'focus' instead of 'focusin'. On FF there is no 'focusin' event.
 2) adding event listener only to the given inputs, not to the base document.

This made FF happy :) will try to work on IE and Opera.

[Ryosuke Niwa]

Comment on attachment 164558 [details]
proposed modification

View in context: https://bugs.webkit.org/attachment.cgi?id=164558&action=review

> Websites/bugs.webkit.org/committers-autocomplete.js:394
> +    function focusHandler(e) {

I would have used anonymous function here.

[Ryosuke Niwa]

That is, I would have done function () {~} in addEventListener.

[Peter Gal]

(In reply to comment #4)
> That is, I would have done function () {~} in addEventListener.

ok I'll update it.

[Peter Gal]

Created attachment 165380 [details]
updated patch

[Ryosuke Niwa]

Comment on attachment 165380 [details]
updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=165380&action=review

> Websites/bugs.webkit.org/committers-autocomplete.js:398
> +        if (field) {
> +            field.addEventListener("focus", function(e) { enableAutoComplete(e.target); }, false);
>          }

No curly brackets around single statement.

[Ryosuke Niwa]

Do you want me to land the patch? If so, please re-upload the patch after a addressing my comment and set cq? as well as r?.

[Peter Gal]

Created attachment 165538 [details]
updated patch #2

"Should be okay now" (TM)

[WebKit Review Bot]

Comment on attachment 165538 [details]
updated patch #2

Clearing flags on attachment: 165538

Committed r129521: <http://trac.webkit.org/changeset/129521>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

I'm no longer getting email completion for the CC field on the "new bug" form.

[WebKit Review Bot]

Re-opened since this is blocked by 97710

[Tony Chang]

On IRC, the cause  of the bug is that enter_bug.cgi doesn't set the ID of the CC <input> and the new code uses getElementById.

You could use a query selector like "input[name=" + EMAIL_INPUTS[i] + "]" instead of getElementById.

[Adam Barth]

(In reply to comment #14)
> On IRC, the cause  of the bug is that enter_bug.cgi doesn't set the ID of the CC <input> and the new code uses getElementById.
> 
> You could use a query selector like "input[name=" + EMAIL_INPUTS[i] + "]" instead of getElementById.

Or just http://www.w3schools.com/jsref/met_doc_getelementsbyname.asp

[Csaba Osztrogonác]

ping? :)

[Ryosuke Niwa]

Created attachment 172520 [details]
Fixes the bug

[Ryosuke Niwa]

Created attachment 172524 [details]
Real patch

Something went awfully wrong in the previous patch.

[Csaba Osztrogonác]

Comment on attachment 172524 [details]
Real patch

LGTM, r=me. I'll land it manually, because CQ is very slow.

[Csaba Osztrogonác]

Comment on attachment 172524 [details]
Real patch

Clearing flags on attachment: 172524

Committed r133576: <http://trac.webkit.org/changeset/133576>

[Csaba Osztrogonác]

All reviewed patches have been landed.  Closing bug.