Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#17725 closed defect (wontfix)

[patch] [cla] uacss is adding class every time it is included

Reported by: iDo Owned by: iDo
Priority: high Milestone: 1.10
Component: Core Version: 1.9.2
Keywords: Cc:
Blocked By: Blocking:

Description

If multiple file are including dojo/uacss, the html get "spammed" by duplicated classnames.

uacss does not check if the class are already added or not.

Change History (7)

comment:2 Changed 5 years ago by dylan

Milestone: tbd1.10
Priority: undecidedhigh

comment:3 Changed 5 years ago by bill

Owner: set to iDo
Status: newpending

No matter how many files "include'" dojo/uacss, it's only loaded once. This is how AMD's define() works. So, I doubt there's any actual bug here. Can you provide a test case showing a failure?

comment:4 Changed 5 years ago by dylan

Assuming there is an issue, it would probably be better to use dojo/dom-class.add (though pulling in that dependency is probably a bit large)... so probably should at least use the same logic we use in dom-class.add if this fix is needed.

comment:5 Changed 5 years ago by Bryan Forbes

Resolution: wontfix
Status: pendingclosed

There is no bug here. The define factory function will only ever run once. The only time I could see this being a problem is if someone is using two versions of Dojo on the same page. However, having the same class multiple times on a node doesn't have an adverse effect: one of the instances is effectively ignored.

With regards to the addition of dojo/dom-class as a dependency, it doesn't really matter. This is a pretty simple module that doesn't really need the code to do replace, toggle, and remove to function.

comment:6 in reply to:  3 Changed 5 years ago by iDo

Replying to bill:

No matter how many files "include'" dojo/uacss, it's only loaded once. This is how AMD's define() works. So, I doubt there's any actual bug here. Can you provide a test case showing a failure?

You're right but dojo give the possibility to load multiple dojo instance in a page. Each instance will add the class. For instance, in our project we have 4 times the class

comment:7 Changed 5 years ago by bill

Ah, OK, yes I didn't think of that case. However, since it's unusual to have multiple dojo instances on a page, and since having a class name repeated is harmless, I would also vote to not fix this.

Note: See TracTickets for help on using tickets.