Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HTML not XHTML compliant #2121

Closed
brianmay opened this issue Nov 24, 2014 · 5 comments
Closed

HTML not XHTML compliant #2121

brianmay opened this issue Nov 24, 2014 · 5 comments

Comments

@brianmay
Copy link

Hello,

I use DEFAULT_CONTENT_TYPE = "application/xhtml+xml" in my code. In fact stuff breaks if I don't use it.

It would be good if I could use it and not break the HTML pages in django-rest-framework. This only affects the login page.

Simple problems I have observed include:

html tag needs to be <html xmlns="http://www.w3.org/1999/xhtml">

Tag namaes should be lowercase; this breaks <Label ...>...</label>

Tags need to be terminated, e.g. <input ... /> instead of <input>.

The quick and dodgy fix would be to add the following to login_base.html, same as base.html:

<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>

This basically overrides my XHTML declaration and says the page is HTML.

Thanks
@brianmay
Copy link
Author

An alternative option, if you decide you only want to support HTML and not XHTML, is to override the content_type for every response, so it doesn't use the DEFAULT_CONTENT_TYPE setting.

This currently appears to be the approach favored by the Django developers at the moment. See a similar bug against Django: https://code.djangoproject.com/ticket/23908#comment:7

I think this would also mean that the meta tag is no longer needed.

tomchristie added a commit that referenced this issue Nov 25, 2014
@tomchristie
Lowercase HTML label tag. Refs #2121.
bdacb66
@tomchristie
Copy link
Member

On master the http-equiv tag is being set on the login view.
Given that's adequate to ensure nothing breaks I'm going to close this off.
Happy to also:

  • Accept PRs making with template improvements, we can then consider them on a case-by-case basis, and perhaps consider moving towards XHTML compliance.
  • Accept PRs that remove the http-equiv tab but ensure that the views do serve the correct (HTML) content type and charset, regardless of any Django settings. I think that'll already always be the case for the browsable API, but it's less obvious how to do that for the login and logout views, which use Django's standard contrib.auth views.

@brianmay
Copy link
Author

The http-equiv tag doesn't work for the login view. After thinking about it, this makes sense, it needs to parse the file before it can find the http-equiv, and by this stage it has already failed.

@brianmay
Copy link
Author

The following patch makes the login page for 3.0.1 XHTML valid. In particular:

  • XML namespace declaration.
  • input fields terminated: <input/> instead of <input>
  • required attribute needs a value (any value will do)
diff --git a/rest_framework/templates/rest_framework/base.html b/rest_framework/templates/rest_framework/base.html
index e966819..c1cfe5e 100644
--- a/rest_framework/templates/rest_framework/base.html
+++ b/rest_framework/templates/rest_framework/base.html
@@ -2,7 +2,7 @@
 {% load staticfiles %}
 {% load rest_framework %}
 <!DOCTYPE html>
-<html>
+<html xmlns="http://www.w3.org/1999/xhtml">
     <head>
         {% block head %}

diff --git a/rest_framework/templates/rest_framework/login_base.html b/rest_framework/templates/rest_framework/login_base.html
index 8e6240a..e3ddde1 100644
--- a/rest_framework/templates/rest_framework/login_base.html
+++ b/rest_framework/templates/rest_framework/login_base.html
@@ -26,8 +26,8 @@
                                         <input type="text" name="username" maxlength="100"
                                             autocapitalize="off"
                                             autocorrect="off" class="form-control textinput textInput"
-                                            id="id_username" required
-                                            {% if form.username.value %}value="{{ form.username.value }}"{% endif %}>
+                                            id="id_username" required="true"
+                                            {% if form.username.value %}value="{{ form.username.value }}"{% endif %}/>
                                         {% if form.username.errors %}
                                             <p class="text-error">
                                                 {{ form.username.errors|striptags }}
@@ -41,7 +41,7 @@
                                         <label for="id_password">Password:</label>
                                         <input type="password" name="password" maxlength="100"
                                             autocapitalize="off" autocorrect="off" class="form-control textinput textInput"
-                                            id="id_password" required>
+                                            id="id_password" required="true"/>
                                         {% if form.password.errors %}
                                             <p class="text-error">
                                                 {{ form.password.errors|striptags }}
@@ -56,7 +56,7 @@
                                     {% endfor %}
                                 {% endif %}
                                 <div class="form-actions-no-box">
-                                    <input type="submit" name="submit" value="Log in" class="btn btn-primary form-control" id="submit-id-submit">
+                                    <input type="submit" name="submit" value="Log in" class="btn btn-primary form-control" id="submit-id-submit"/>
                                 </div>
                             </form>
                         </div>

@tomchristie
Copy link
Member

if you decide you only want to support HTML and not XHTML

This would by my preferred option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@brianmay @tomchristie