Opened 10 years ago

Closed 10 years ago

#3693 closed Bug (wontfix)

dom.element.getAttribute returns unnormalized style properties

Reported by: Tobiasz Cudnik Owned by: Tobiasz Cudnik
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: Confirmed Review- IE webkit Cc:

Description

In some cases IE8 and webkit returns uppercases css properties after getAttribute('style'). To prevent this, place for browser-wide code should be introduced to this method and there the css normalization could be done.

Attachments (1)

3693.patch (1.7 KB) - added by Tobiasz Cudnik 10 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: newassigned

Changed 10 years ago by Tobiasz Cudnik

Attachment: 3693.patch added

comment:2 Changed 10 years ago by Tobiasz Cudnik

Keywords: Confirmed Review? IE webkit added

comment:3 Changed 10 years ago by Garry Yao

Keywords: Review- added; Review? removed

The jQuery chaining coding style is convention compliant at L411, which means line break should not happen before dot.

comment:4 Changed 10 years ago by Frederico Caldeira Knabben

There fix impacts on performance. Can you point use to a practical usage of it? Do we have any issue in the editor that is related to it?

comment:5 in reply to:  3 Changed 10 years ago by Frederico Caldeira Knabben

Replying to garry.yao:

The jQuery chaining coding style is convention compliant at L411, which means line break should not happen before dot.

For that case, the coding style helps on readability, so it should be acceptable (this rule is also part of our coding styles).

comment:6 Changed 10 years ago by Garry Yao

It's used by L730 of styles plugin, and actually, there's very few places of our codebase is retrieving 'style' attribute, so PF would not be a big problem.

				// The 'class' element value must match (#1318).
				if ( attName == 'class' && element.getAttribute( attName ) != attributes[ attName ] )
					continue;

comment:7 in reply to:  6 Changed 10 years ago by Frederico Caldeira Knabben

Replying to garry.yao:

It's used by L730 of styles plugin

:? Sorry but, L730 is talking about the "class" attribute, and this ticket is related to the "style" attribute.

comment:8 Changed 10 years ago by Tobiasz Cudnik

So what's the status of this ticket ? If this fix is unnecessary then TC element.html should be fixed, because it relays on exact results from getAttribute( 'style' ).

comment:9 Changed 10 years ago by Frederico Caldeira Knabben

Resolution: wontfix
Status: assignedclosed

We don't need to fix things simply to make TCs pass. They must be useful, and they must mainly impact on the editor features. Otherwise we instead have TCs to get fixed.

If you see any part of the code that may be impacted by this, then please reopen this ticket with more details.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy