Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1998 (closed defect: fixed)

Opened 9 years ago

Last modified 8 years ago

outputformat="xhtml" is incompatible with some browsers.

Reported by: ondrejj Owned by: chrisz
Priority: normal Milestone: 1.0.x bugfix
Component: TurboGears Version: 1.0.7
Severity: normal Keywords:
Cc:

Description

Updating to TurboGears-1.0.7 breaks compatibility with IE browsers. Also IE7 is does not know anything about application/xhtml+xml types. When trying to open this page, it only gives me ability to save this page.

I know, that w3c standards suggest to set this content type for XHTML pages, but it is only a "should" and not "must".

Also there is a small problem, that in Firefox 3 these pages works differently. For example trying this:

http_response.innerHTML = http_request.responseText;

if fails, if responseText is not a valid xhtml or it can't be inserted into specified tag (for example if it contains <body> or similar tags. If someone need help, there is a proper fix:

http_response.innerHTML = http_request.responseXML.getElementsByTagName('body')[0].innerHTML;

I think it is not a good idea to break compatibility in bugfix release. May be breaking compatibility in TG2 can be possible, but bugfix release which changes application behaviour is a very bad idea.

At least add an config option to disable this "feature". Before this version it worked on tg-1.0.4.4.

Change History

comment:1 follow-up: ↓ 3 Changed 9 years ago by chrisz

Since you say "also IE does not know about app/xhtml+xml", do you want to say that there are also other problems with IE, or should I read it as "because IE does not know about app/xhtml+xml", so this is only this one issue? I assume the latter.

And I agree, this is a serious issue and it needs to be fixed in TG 1.0.8. Most people are probably using TG's default html format, that's why nobody cried earlier.

I made this change in r5322 without rethinking the consequences, relying on the W3C recommendation only and believing IE has improved in the last versions. But you're right about the problems with application/xhtml+xml: IE still doesn't understand it, and Mozilla automatically goes into "full standards mode" without looking at the doctype.

I see several options how to fix this:

  1. Simply revert the change, always send 'text/html' for both html and xhtml.
  2. Make the content type for xhtml configurable, with the old 'text/html' as default.
  3. Make the whole mapping from template formats to content types (as used in view.base.render) configurable, with 'text/html' as default for xhtml.
  4. Use content negotiation (based on the Accept header, not the User-Agent-Header!). But I fear this will lead into even more problems.

comment:2 Changed 9 years ago by chrisz

Sorry, the critical change was in r5323, not 5322. I think in such cases we should make explicit comments in the code that e.g. using text/html for xhtml was not an oversight, but the desired behavior, so that the next one will not "fix" this again.

comment:3 in reply to: ↑ 1 Changed 9 years ago by ondrejj

Replying to chrisz:

Since you say "also IE does not know about app/xhtml+xml", do you want to say that there are also other problems with IE, or should I read it as "because IE does not know about app/xhtml+xml", so this is only this one issue? I assume the latter.

Sorry, wrong formulation. I know about:

IE6, IE7: Don't work.
FF3, Opera8: Works.

And I agree, this is a serious issue and it needs to be fixed in TG 1.0.8. Most people are probably using TG's default html format, that's why nobody cried earlier.

Thank you.

I see several options how to fix this:

  1. Simply revert the change, always send 'text/html' for both html and xhtml.
  2. Make the content type for xhtml configurable, with the old 'text/html' as default.
  3. Make the whole mapping from template formats to content types (as used in view.base.render) configurable, with 'text/html' as default for xhtml.
  4. Use content negotiation (based on the Accept header, not the User-Agent-Header!). But I fear this will lead into even more problems.

Fix4 looks better for me, but all fixes can be good. May be autonegitiation with configurable overwrite can be good too. As with this content type Firefox is working different way, I know now, that for FF it's a proper way and application/xhtml+xml is a feature.

comment:4 Changed 9 years ago by Chris Arndt

  • Owner changed from faide to chrisz

comment:5 Changed 9 years ago by chrisz

The problem with content negotiation is that we really need to do it right, i.e. check the accept header including the qs parameters, plus maybe even some exceptions for certain user agents. Another problem is that content negotiation can lead to problems with proxies/caches, e.g. when a page has first been requested by a Firefox browser and then by a IE browser which gets the page with the wrong content type from the cache, though I don't know how serious that caching problem really is. I just read a helpful  article (unfortunately in German only) that advises against content negotiation because of the complexity and problems.

So I think we should at least not introduce content negotiation in TG 1.0, but for the time being, we should resort to a simple solution such as 2. or 3. Maybe we should open a new trac ticket with the suggestion of adding this as a feature in TG 1.1 or 1.5.

comment:6 Changed 9 years ago by Chris Arndt

Yes, content negotiataion is evil. IMHO a URI (together with a request method) should be unambigous. Conneg goes against that.

comment:7 Changed 9 years ago by ondrejj

OK, agree.

I just want to tell, that negotiation is useful.

All other solutions are good too.

Btw, I think we need to change milestone to 1.0.x bugfix.

comment:8 Changed 9 years ago by Chris Arndt

  • Milestone changed from 1.1 to 1.0.x bugfix

comment:9 Changed 8 years ago by chrisz

I'll be away this weekend, but I'll work on it next week when nobody else picks it up.

comment:10 Changed 8 years ago by chrisz

  • Status changed from new to closed
  • Resolution set to fixed

Implemented option 3 in r5548. Sorry for the delay.

Note: See TracTickets for help on using tickets.