572696 - Fennec should understand meta viewport with spaces

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Anthony Ricaud (:rik)]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_4; fr-fr) AppleWebKit/534.1+ (KHTML, like Gecko) Version/5.0 Safari/533.16
Build Identifier: http://people.mozilla.com/~mbrubeck/fennec.apk

Fennec doesn't understand <meta name="viewport" content="width = device-width">

Most of Apple documentation is using this syntax with spaces so there's potentially a lot of copy/paste pages.

See http://developer.apple.com/safari/library/documentation/appleapplications/reference/safariwebcontent/usingtheviewport/usingtheviewport.html#//apple_ref/doc/uid/TP40006509-SW26 for examples.

Reproducible: Always

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

This is not a fennec bug, this is a platform bug.
The patch should do the work but I need to test it.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

It was a bit more complicated than what I was thinking first.

The patch allow spaces around the '=' character but still allow to use ' ' as a separators between tokens to keep cross browsers compatibilities.

Comment 3

[Mounir Lamouri (:mounir)]

I think you should write tests for that. It shouldn't be hard.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #3)
> I think you should write tests for that. It shouldn't be hard.

We have some tests already. And will add new ones for this:
http://mxr.mozilla.org/mobile-browser/source/chrome/tests/browser_viewport.js

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2 - rebased on trunk

Patch updated on trunk

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Very simple tests

Basic tests for checking spaces, I should probably add checks for "\t", "\r", "\n" too

Comment 7

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/99de996537ef

Keeping open given that tests need to be pushed on mobile-browser.

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Tests: 
http://hg.mozilla.org/mobile-browser/rev/655367cfcc57

Thanks Mounir for landing the platform part!