defined this.input but never get use ?? pure access to ID is more better.

(Version: should I point to D8 and port back ?)

CommentFileSizeAuthor
#5 core-js-autocomplete-id-1099062-5.patch480 bytesnod_
#5 core-js-autocomplete-id-1099062-5-D7.patch460 bytesnod_
autocomplete.patch452 bytesdroplet

Comments

catch’s picture

catch
he/him
Primary language English
commented
Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

Looks sensible enough, we miss a js testing framework for patches like this.

nod_’s picture

nod_
Primary language French
Location Lille
commented

I don't really see the use of this, there is virtually no impact on performance. Optimize a selector somewhere and you get tens or hundreds of times the performance boost this gives you.

The input is used later in the code. Drupal.jsAC is a constructor, it's not just a function, the comment is misleading.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +JavaScript clean-up

Turns out I was wrong : http://jsperf.com/el-attr-id-vs-el-id/2 It does matter. Might want to look elsewhere too.

Selector improvement is a bigger win though.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

That is a pretty cool test. This patch doesn't seem to apply though.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Needs review
StatusFileSize
new core-js-autocomplete-id-1099062-5-D7.patch460 bytes
new core-js-autocomplete-id-1099062-5.patch480 bytes
Tor Arne Thune’s picture

Tor Arne Thune commented
Status: Needs review » Reviewed & tested by the community

Applies now.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x and 7.x. Thanks for the quick re-roll.

droplet’s picture

droplet commented

Thanks. It's waited 10 months.

created a follow up issue for other parts:
#1099062: autocomplete.js micro optimization

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D7, -JavaScript clean-up

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Needs backport to D7, +JavaScript clean-up

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.