remy / inliner

Node utility to inline images, CSS and JavaScript for a web page - useful for mobile sites
MIT License
1.1k stars 165 forks source link

Use of a 'simpler' DOCTYPE #33

Closed jamesplease closed 9 years ago

jamesplease commented 11 years ago

One of the features of inliner is that it uses a 'simpler' DOCTYPE. It is true that it uses a shorter one, but changing the DOCTYPE has no benefits that I know of, and has negative consequences for email development.

The DOCTYPE it forces you to use isn't just a simpler one, as it's also, of course, the DOCTYPE for HTML5 documents. This is the latest DOCTYPE, but in this case there is hardly any gain in using it. In fact, both emailonacid and campaign monitor suggest using the xhtml1 DOCTYPE when it comes to email. But they also leave it open to the developer to choose what they'd like to use, as your choice affects how the email renders in numerous clients.

On the side of web development, why force someone over to a new DOCTYPE if they're not building an HTML5 site? Sure, there's no harm done, but there's also no benefit.

The point I'm trying to make is that switching to a 'simpler' DOCTYPE probably isn't a good idea for this project, given that email developers might be developing around other DOCTYPES, and I can't think of any benefits of this feature.

Suggestion: Make the feature optional, at least. Personally, I wouldn't even have it turned on by default. Just leave the DOCTYPE the way it is.

spacez320 commented 11 years ago

Wouldn't it make.more sense to just use the doctype provided in the original document?

jamesplease commented 11 years ago

Yup. I agree, @spacez320. I thought for sure I had explicitly stated it, but upon reading it again it seems it was only slightly implied, at best. I've updated it to be more clear. Thanks!

remy commented 11 years ago

Just for the record, the reason the doctype is overwritten (IIRC - it's been a long time) is becasue jsdom doesn't give me access to the doctype, so it's lost in the conversion. I suspect a simple regexp on the original document might fix it, but not completely sure.

On 4 October 2013 12:27, James Smith notifications@github.com wrote:

Yup. I agree, @spacez320 https://github.com/spacez320. I thought for sure I had explicitly stated it, but upon reading it again it seems it was only slightly implied, at best. I've updated it to be more clear. Thanks!

— Reply to this email directly or view it on GitHubhttps://github.com/remy/inliner/issues/33#issuecomment-25692276 .

remy commented 9 years ago

This is fixed @ 1.0.0.

jamesplease commented 9 years ago

:trumpet: