RitsC / PrintArea

jQuery plugin to print specified area of a page.
Other
264 stars 163 forks source link

HTML5 and IEedge support #6

Closed jer-gallagher-oe closed 10 years ago

jer-gallagher-oe commented 11 years ago

-Fixed unhandled error when parsing CSS links without optional "media" attribute (line 104)

RitsC commented 11 years ago

Can you please explain the nature of the media "bug"? I am not getting an exception regarding this filter.

The intent is that if the rel attribute is 'stylesheet' and the media attribute is either [undefined, empty, 'print', 'all'] it gets included in the printed document. The reason is that empty and undefined media is indeterminate and should be excluded based on explicit evaluation.

jer-gallagher-oe commented 11 years ago

Sure, since the "media" attribute is optional on the html LINK element, sometimes jquery doesn't return a string for the "media" variable, which in turns throws a javascript error when calling a string method like media. toLowerCase() (am able to repro in IE9)

RitsC commented 11 years ago

Hmmm... yes, I thought that may be what you were referring to, however, media == undefined accounts for that situation, unless you have defined what undefined is elsewhere.

RitsC commented 11 years ago

I'll take a look, I was using IE10 to test.

RitsC commented 11 years ago

I am not able to reproduce error in IE9. Downloaded a VM with IE9 (http://www.modern.ie/en-us/virtualization-tools#downloads), copied the demo package and cannot reproduce using the demo.

jer-gallagher-oe commented 11 years ago

To repro, remove REL attribute from css link tags like this: < link type="text/css" href="empty.css" /> < link type="text/css" href="noPrint.css" /> < link type="text/css" href="media_all.css" media="all" /> < link type="text/css" href="media_none.css" media="xyz" />

In IE devtools console observe this error message: SCRIPT5007: Unable to get value of the property 'toLowerCase': object is null or undefined jquery.PrintArea.js, line 92 character 33

RitsC commented 11 years ago

Yes, I purposefully created the demo (a long time ago) with those links where the media attribute is absent to test for this type of issue. In a stock IE9 browser the demo works fine, I am not seeing this error using the built-in IE Developer Tools (F12).

You might add this line after the media variable is created to get some insight to the issue. alert(" media: " + media + " \n undefined: " + undefined + " \n equal? " + (media == undefined) ) Or you could also use the javascript debugging console in Developer Tools in IE to see why the conditional "or" is being passed, in other words, why media == undefined is evaluating as false

A simple fix may be to replace media == undefined with !media.

RitsC commented 11 years ago

I'm working on a cumulative fix for outstanding bugs/issues. I will have something more dependable at that time.

jer-gallagher-oe commented 11 years ago

The problem is that the filter line 92 expects a"rel" attribute: $(this).attr("rel").toLowerCase()

RitsC commented 11 years ago

Looks like the conversation switched a while back... from "media" to "rel", and I missed it. Ok, the "rel" attribute is required according to http://www.w3schools.com/tags/tag_link.asp, however I will include a check for that also.

Thanks for all the input.

jer-gallagher-oe commented 11 years ago

The "media" variable is impacted because of the prior jquery filter using "rel", so the falsy test if(media) handles this.

Btw, the "rel" attribute is required only in HTML5, but to be backwards compatible it is optional in 4.0.1. http://www.w3.org/TR/html401/present/styles.html#style-external

The W3C defines the type attribute for external css pages... 12.3.2 Links and external style sheets When the LINK element links an external style sheet to a document, the type attribute specifies the style sheet language and the media attribute specifies the intended rendering medium or media. User agents may save time by retrieving from the network only those style sheets that apply to the current device http://www.w3.org/TR/html401/struct/links.html#edef-LINK

RitsC commented 11 years ago

Thanks, I am aware of all of that information.

RitsC commented 10 years ago

The [rel="stylesheet"] attribute will be required criteria for the 'link' to be included. You can use either the option 'extraHead' to include links that do not have this attribute or the option 'extraCss'.

Also, you will want to use the 'extraHead' option to include the 'meta' tags needed for IEedge.

I want to keep this plugin as simple as possible without interpreting standards and without over complicating configurations.