Bug Tracker

Modify

Ticket #2671 (closed bug: patchwelcome)

Opened 6 years ago

Last modified 3 years ago

curCSS: computed style hack for IE

Reported by: linwong Owned by:
Priority: low Milestone: 1.next
Component: css Version: 1.5.2
Keywords: Cc:
Blocking: Blocked by:

Description

I get a computed error for line-height because of this hack.

In my case, I had a line height of 1.6 (which is a valid value). Upon trying to calculate the pixels for this, it returns 1px, which is not correct.

I got the error using .css

$("p").css("line-height");

returned 1px when my line-height was 1.6

Attachments

lineheight.jquery.html Download (728 bytes) - added by kizu 5 years ago.
Test Case for the bug

Change History

comment:1 Changed 6 years ago by flesler

  • need changed from Review to Test Case
  • Milestone changed from 1.2.3 to 1.2.4

Could you add a test case for this ?

comment:2 Changed 5 years ago by sanaell

Seems fixed case: alert($("#result").css("line-height")); return normal $("#result").css("line-height",1.5); alert($("#result").css("line-height")); return 1.5

under Chrome,Internet explorer

comment:3 Changed 5 years ago by sanaell

sorry tested again

--- to fix that problem need to fix ATTR function in core.js with that * old line *

if ( typeof name === "string" )

if ( value === undefined ) {

return this[0] && jQuery[ type
"attr" ]( this[0], name );

change for *

if ( typeof name === "string" )

if ( value === undefined ) {

var camelCase = name.replace(/\-(\w)/g, function(all, letter){

return letter.toUpperCase();

});

return this[0] && jQuery[ type
"attr" ]( this[0], camelCase );

}

Changed 5 years ago by kizu

Test Case for the bug

comment:5 Changed 4 years ago by dmethvin

  • Component changed from core to css

Still a problem with IE8 in 1.4.2 so I'll leave it open.

comment:6 Changed 4 years ago by snover

  • Status changed from new to pending

This ticket has been marked as missing a test case. In an effort to reduce the number of outstanding tickets in the bug tracker, it will be closed automatically in 30 days. In order to prevent this from happening, please provide a working test case. If a test case has already been provided and our records are wrong, please respond to the ticket so that it can be fixed. Thank you!

comment:7 Changed 3 years ago by trac-o-bot

  • Status changed from pending to closed

Automatically closed due to 14 days of inactivity.

comment:8 Changed 3 years ago by ian@…

I've verified that this issue exists still in jQuery 1.5.2, and have updated the test-case:

 http://jsfiddle.net/qqyqM/

Particularly worrisome is the fact that on IE (I tested IE8 specifically), a line-height of 1.5 is converted by jQuery into "1px".

Can this bug be re-opened?

comment:9 follow-up: ↓ 10 Changed 3 years ago by timmywil

@ian - unfortunately that's what IE actually computes for line height when you do not supply a unit. jQuery cannot control this. Use "em".

comment:10 in reply to: ↑ 9 Changed 3 years ago by Ian Gilman <ian@…>

Replying to timmywil:

@ian - unfortunately that's what IE actually computes for line height when you do not supply a unit. jQuery cannot control this. Use "em".

Actually that's not the case; IE's .currentStyle has the correct value even in that case. I've updated the testcase to show this:

 http://jsfiddle.net/qqyqM/4/

comment:11 follow-up: ↓ 12 Changed 3 years ago by timmywil

Version 0, edited 3 years ago by timmywil (next)

comment:12 in reply to: ↑ 11 Changed 3 years ago by Ian Gilman <ian@…>

Replying to timmywil:

That's not what I get:  http://jsfiddle.net/timmywil/qqyqM/7/

With that testcase, I get this console output:

css:  1
currentStyle:  1
css:  1px
currentStyle:  1.5

This is with IE 8.0.7600.16385 running on Windows 7.

What result are you getting? What version of IE?

comment:13 Changed 3 years ago by timmywil

That's what I get, but that's what I was saying. IE8 does not return the correct value with currentStyle. Adding em fixes the issue tho.

comment:14 Changed 3 years ago by Ian Gilman <ian@…>

I see, because you want it to return the correct value in pixels. I agree that would be great, but short of that, I feel it should at least return the actual value, rather than mangling it. The current state of the code means that this:

$(foo).css("line-height", $(foo).css("line-height"));

... is an unsafe operation, because it'll convert 1.5 to 1px, rather than just passing it straight through. It seems to me that we should strive for that sort of round-trip to always be safe.

I agree that adding em is an easy enough work-around, except that:

  • People who haven't read this bug thread won't necessarily know to do it.
  • Sometimes you don't have control over the CSS. For instance, I ran across this issue because I'm working on a bookmarklet that needs to operate on arbitrary web pages. Both slashdot and google search results have line-height values that fit this pattern (1.x with no em).

I wouldn't say it's a high-priority bug, but I do believe it's a valid bug.

comment:15 Changed 3 years ago by timmywil

  • Priority changed from minor to low
  • Version changed from 1.2.3 to 1.5.2
  • Milestone changed from 1.2.4 to 1.next

My argument is that conversion from em to px is the browser's job. Unfortunately, IE doesn't understand that the user means means em when no unit is provided and simply passes through the set value. jQuery cannot guarantee that the operator intends em either. Most often, browsers do make that assumption, but there's also cm, mm, in, pt, pc, or ex, which is a mixture of absolute unit and relative unit designators. And if we were to assume em, the potential fix would be either to set em for the user ourselves before getting the computed value (which could be unpredictable and slow down .css) or compute to px ourselves (which would be expensive and probably not always be accurate, especially given transforms and zoom). So I'm not sure there's a workable solution to the problem.

comment:16 Changed 3 years ago by Ian Gilman <ian@…>

Sounds like we agree that it's not worth it/possible to properly convert to pixels.

I do believe it's worth it to return the actual currentStyle value rather than mangling it. This already works for integer values (with 1 being returned as 1), so it just needs to be fixed for non-integer values (so 1.5 is returned as 1.5 rather than 1px).

I don't have permissions on this bug database to re-open... is that something you can do, Tim?

Thank you.

comment:17 follow-up: ↓ 20 Changed 3 years ago by timmywil

I think we might accept a patch, but I don't foresee this being something we're going to tackle.

comment:18 Changed 3 years ago by timmywil

  • Status changed from closed to reopened

comment:19 Changed 3 years ago by timmywil

  • Status changed from reopened to closed
  • Resolution set to patchwelcome

comment:20 in reply to: ↑ 17 Changed 3 years ago by Ian Gilman <ian@…>

Replying to timmywil:

I think we might accept a patch, but I don't foresee this being something we're going to tackle.

Fair enough; thank you.

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.