Bug 39148 – Handle <font size=0> as <font size=1> like any other browser

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[j.j.]

Any other browser engine handles

<font size="0"> the same way as <font size="1">

WebKit ignores the size, so it displays much too large (like size="3").

I came from time to time on a site with that bug, but have no URL at hand just now.

[j.j.]

Created attachment 56135 [details]
testcase

[Alexey Proskuryakov]

*** This bug has been marked as a duplicate of bug 11004 ***

[j.j.]

I read bug 11004 before filing this, and don't think it's a dup.

My bug requests  <font size=0>  should be the same as <font size=1>
and therefore should map to CSS  |font-size: xx-small|  or something.

Bug 11004, as I understand it, complains that CSS  |font-size: 0px|
should result in invisible text, not in 1px sized text. That's an unrelated issue.

[Alexey Proskuryakov]

Bug 11004 says "should make the text not render at all (Firefox) or no bigger than 1px (WinIE)".

But it indeed talks about CSS font-size, not <font size=0>, which I overlooked. Let's re-open for now, and wait for rendering experts' comments.

[j.j.]

(In reply to comment #4)
> Bug 11004 says "should make the text not render at all (Firefox) or no bigger than 1px (WinIE)".

Here we have a converse problem:
<font size=0> is rendered too big (like size=3) and should be smaller like <font size=1>.
Not much of expertise necessary ;) 

Note also:
"0" is an invalid value for "size" in HTML4 and behaviour is therefore undefined per spec. It just an issue of compatibility. 
(May be there is some text in HTML5, don't know)

[j.j.]

This bug violates HTML5 UA requirements
http://dev.w3.org/html5/spec/rendering.html#fonts-and-colors

 "When a font element has a size attribute ...
   ... 
   11.  If value is less than 1, let it be 1."

[Adam Barth]

Looks like Darin introduced the current behavior in http://trac.webkit.org/changeset/1655 (which was 8 years ago).

[Darin Adler]

Yes, seems fine to make our behavior match the other browsers. I didn’t know a lot about web browsers 8 years ago.

[Adam Barth]

Created attachment 67519 [details]
Patch

[Eric Seidel (no email)]

Attachment 67519 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4041002

[Adam Barth]

Created attachment 67520 [details]
Patch

[Eric Seidel (no email)]

This isn't a security bug.

[Darin Adler]

Comment on attachment 67520 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67520&action=prettypatch

Thanks for tackling this. Like other the many other HTML-specification-adoption changes we are making these days, there is almost certainly going to be some compatibility fallout in the future from content that has only ever been tested with WebKit. So we should be on the lookout for those regressions.

> WebCore/html/HTMLFontElement.cpp:60
> +        if (!isSpaceOrNewline(*position))
> +            break;
WTF’s isSpaceOrNewline function is not correct to implement the HTML specification’s “skip whitespace” algorithm. That is supposed to skip “space characters” and the function isHTMLSpace in HTMLParserIdioms.h is the right one for that. I’d like to see a test case or two covering that difference.

r=me but please use isHTMLSpace rather than isSpaceOrNewline.

[Adam Barth]

> > WebCore/html/HTMLFontElement.cpp:60
> > +        if (!isSpaceOrNewline(*position))
> > +            break;
> WTF’s isSpaceOrNewline function is not correct to implement the HTML specification’s “skip whitespace” algorithm. That is supposed to skip “space characters” and the function isHTMLSpace in HTMLParserIdioms.h is the right one for that. I’d like to see a test case or two covering that difference.

Will do.

[Adam Barth]

I haven't tested, but I suspect this will actually be a compat progression on some versions of PeopleSoft (in particular, the version used by UC Berkeley).

[Darin Adler]

(In reply to comment #15)
> I haven't tested, but I suspect this will actually be a compat progression on some versions of PeopleSoft (in particular, the version used by UC Berkeley).

Sure, I expect compatibility progressions on many sites that do not have WebKit-specific code!

[Adam Barth]

Created attachment 67596 [details]
Patch for landing

[Adam Barth]

Comment on attachment 67596 [details]
Patch for landing

Clearing flags on attachment: 67596

Committed r67514: <http://trac.webkit.org/changeset/67514>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/67514 might have broken Qt Linux Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/67513
http://trac.webkit.org/changeset/67514