311569 - font-size within style attributes does not work without units

Status: RESOLVED WONTFIX

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051006 Firefox/1.4.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051006 Firefox/1.4.1

Although mozilla supports font-size="12.1" for <text> elements it does not work
if you define it as <text style="font-size: 12.1;">test</text>


Reproducible: Always

Steps to Reproduce:
1. go to http://telemetry.logica.co.uk
2. login using the default password
3. The displayed page should have text with various different font sizes

Actual Results:  
All the text has the same font-size and you get warnings in the javascript console

Expected Results:  
displayed the text with the correct font-size

Comment 1

[Robert Longson [:longsonr]]

Attachment: let SVG have its own copy of ParseStyleAttribute which sets the SVG flag on the css parser

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 198844 [details] [diff] [review]
let SVG have its own copy of ParseStyleAttribute which sets the SVG flag on the css parser

This is too much code duplication. Instead just hack the normal
ParseStyleAttribute to check the namespace of the element and enable svgmode
for svg elements.

Comment 3

[Robert Longson [:longsonr]]

Attachment: Replaces original patch to address comments by Jonas

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 199319 [details] [diff] [review]
Replaces original patch to address comments by Jonas

Alternativly you could do

PRBool isSVG = ns == SVG
if (isSVG)
  SetSVGMode(PR_TRUE)

...

if (isSVG)
  SetSVGMode(PR_FALSE)

To save yourself two virtual calls in the common case. No biggie though.

Comment 5

[Robert Longson [:longsonr]]

Attachment: address review comments

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 199405 [details] [diff] [review]
address review comments

sr=jst

Comment 7

[tor]

Checked in for Robert.

Comment 8

[Hixie (not reading bugmail)]

Uh. This should not have gone in, IMHO. style="" contains text/css, and in text/css we should not be accepting unitless lengths, since they are ambiguous and syntactically incorrect.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'd agree that this should be backed out, for a number of reasons:

It's getting pretty close to a slippery slope towards allowing this behavior on non-SVG content in two ways:
 1) The difference between style attributes and style elements is smaller than the difference between SVG attributes and style attributes.
 2) style attributes may be extended to allow selector syntax within, and those selectors could match non-SVG content.
And I really don't want to get anywhere close to allowing unitless lengths for non-SVG.

In addition, there are numerous parsing problems that this introduces (which are not a problem in the SVG attributes since they don't have shorthands), and it also makes things that are clearly text/css not conformant to CSS.

So I'd prefer not to do this, especially since we'd be committing ourselves to it permanently if we released something with this change.

I'm also curious why it wasn't reviewed by any style system peer or owner.

Comment 10

[Robert Longson [:longsonr]]

Attachment: testcase

Comment 11

[Robert Longson [:longsonr]]

Ian, David,

Is it possible to recode this fix to meet your concerns while still displaying svg correctly? Do you have any suggestions if so?

It does seem fairly common for svg authors to have done this. Bug 313829 illustrates this as do many of the examples at http://pilat.free.fr/english/routines/js_dom.htm.

Robert

Comment 12

[Scooter Morris]

Ian, et. al.,

I understand and can appreciate the concerns that this patch brings up, but I think we should allow this single patch since it breaks so much existing content.  If the content in question was only hand-authored content, I would take the position that authors should just fix their content to be standards-compliant.  Unfortunately, much of the content in question is generated by applications, which just makes our SVG implementation look wrong.  It's very unfortunate that the SVG WG hasn't clarified this conflict between the CSS specification and the SVG specification, but they haven't (and probably won't).  Since this patch limits the parsing changes to style attributes on SVG content -- I think we should allow it to go forward.  It will really have a significant beneficial impact on our ability to render existing SVG content.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'd prefer to back it out for now and maybe reconsider when we get closer to 1.9.  It's worth noting that the behavior without the patch is probably more likely to be a realistic compromise than the behavior with it.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(And this definitely shouldn't have been done without auditing the CSS parser code for all the bugs this could trigger, e.g., with shorthands.)

Comment 15

[Hixie (not reading bugmail)]

There isn't much SVG content out there (relative to, e.g., the amount of SVG content). Mozilla is in a position to force SVG content producers to fix their content to not be so broken. As with the namespace issue, this is our opportunity to help the Web get out of the Tag Soup mess. Let's not encourage it by supporting this kind of content.

Yes, I know there will be growing pains. But on the long term this benefits everyone.

Comment 16

[Boris Zbarsky [:bzbarsky]]

I assume Ian meant relative to HTML content....

For what it's worth, I agree with David and Ian -- as long as SVG claims that the value of the style attribute is CSS, we should be treating it as CSS.

Comment 17

[Scooter Morris]

I guess I'm OK with holding to the principle if there is an agreement with the other native SVG projects like WebKit+SVG and Opera.  If there seems to be a unified stance on this, then the authoring tools (photoshop, illustrator, visio, etc.) will change and use attributes rather than inline CSS styles.  If there is diversity in the community, then we're just going to frustrate authors and users alike.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Ian, Boris, and I agree that this should be backed out, so I've backed it out.  The discussion can continue, but the burden shouldn't be on us to remember that something needs to happen here when it never should have gone in without our approval in the first place.  (I've had this bug open in a browser window for the past two days so I don't forget about it, and I shouldn't have to keep doing that.)

Comment 19

[Scooter Morris]

(In reply to comment #18)
> Ian, Boris, and I agree that this should be backed out, so I've backed it out. 
> The discussion can continue, but the burden shouldn't be on us to remember that
> something needs to happen here when it never should have gone in without our
> approval in the first place.  (I've had this bug open in a browser window for
> the past two days so I don't forget about it, and I shouldn't have to keep
> doing that.)
> 
No problem, and I agree that it should not have gotten in without a module owner.  By the way, I checked and Visio's SVG output and it definitely will not render in Gecko because it generates a bunch of text/css that uses unitless font sizes.

Comment 20

[Elmar Ludwig]

*** Bug 319162 has been marked as a duplicate of this bug. ***

Comment 21

[Elmar Ludwig]

*** Bug 313829 has been marked as a duplicate of this bug. ***

Comment 22

[Robert Longson [:longsonr]]

*** Bug 330556 has been marked as a duplicate of this bug. ***

Comment 23

[Boris Zbarsky [:bzbarsky]]

Note that per the latest SVG 1.2 status this bug is invalid (at least as far as 1.2 is concerned).

Comment 24

[Martin von Gagern]

The SVG 1.1 specification even contained this method as an example:
http://www.w3.org/TR/SVG11/coords.html#Units
It also went to some length to describe handling of lengths without units:
http://www.w3.org/TR/SVG11/text.html#propdef-font-size
And generally XML attributes are consideret equivalent to CSS properties:
http://www.w3.org/TR/SVG11/styling.html#SVGStylingProperties

So at least for SVG 1.1 we should address this issue, and not considere documents using this technique broken. And I guess SVG 1.1 will be around for some time yet.

(In reply to comment #23)
> Note that per the latest SVG 1.2 status this bug is invalid
> (at least as far as 1.2 is concerned).

Where did you find this, exactly?

For SVG 1.2 they indeed changed the Units example to use the presentation attribute instead of a CSS property:
http://www.w3.org/TR/SVGMobile12/coords.html#Units
They also removed the paragraph about font sizes without length:
http://www.w3.org/TR/SVGMobile12/text.html#propdef-font-size

But the example still uses a font size without unit specification, and still presentation attributes and CSS properties are deemed equivalent:
http://www.w3.org/TR/SVGMobile12/styling.html#SVGStylingProperties

Both in SVG 1.1 and SVG 1.2 the CSS2 spec is considered normative except stated differences. So you could consider the removal of this paragraph as an indication to apply the CSS <length> definition, which requires a unit specifier. In this case, the example would be broken, even though it does not use CSS.

On the whole I guess the SVG 1.2 spec is unclear and probably inconsistent. But as SVG 1.1 requires accepting font sizes without unit, let's have this patch and be happy, except for maybe someone writing to the SVG lists about this problem.

One minor thing about the comments in the attached patches. font-size:5 would be equivalent font-size:5px. Mentioning font-size:5pt instead could lead to some confusion, so I suggest changing this.

Comment 25

[Martin von Gagern]

I did some further reading. Section 6.6 of TinySVG 1.2 states that SVG shares data type definitions with CSS, but that there is indeed a difference for the <length> data type: http://www.w3.org/TR/SVGMobile12/styling.html#StylingSVGUseOfCSS
To me it is still not clear wether this change of definition would apply to CSS used by SVG or only to presentation attributes. At least, given this definition, the example in the 1.2 spec is correct.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Please do actually read this bug, instead of just commenting.  The comments clearly explain _why_ what SVG 1.1 says is not desirable (and even the SVG working group is convinced of this, as you noted in your reading of SVG 1.2).  If given the choice between violating the CSS spec and violating the SVG 1.1 spec, we're going to violate the latter, given that.

Comment 27

[Robert Longson [:longsonr]]

Resolving as WONTFIX based discussion above. If SVG 1.2 appears and confirms this (comment 23) then the status could change to INVALID.

Comment 32

[Kumaran Santhanam]

Thank you, Robert, for bringing this original bug to my attention.

I would like to make two responses to comments from above:

[comment 15, posted in 2005]
"Mozilla is in a position to force SVG content producers to fix their content to not be so broken."

Given that nearly 5 years have passed with no content provider paying heed to how Mozilla renders SVG, I would say that this statement has been conclusively proven to be false.  This is the kind of position that I would expect from a certain other commercial browser vendor, not from the Mozilla community. :-)


[comment 26, posted in 2006]
"If given the choice between violating the CSS spec and violating the SVG 1.1 spec, we're going to violate the latter, given that."

This is not a mutually-exclusive decision.  It should be possible to support the SVG 1.1 font-size specification without affecting global CSS for other subsystems.


I'd like to say that I'm disappointed with the way that this issue has been handled.  There have been 4 other instances (including my recent posts) in which users have bumped up against this issue.  The SVG content community is not using Mozilla as the standard for rendering.  It's time for Mozilla to step up and join the de facto standard of all other renders, not to mention the SVG 1.1 spec.

Despite your philosophical disagreements, the SVG 1.1 spec is published and in use.  Content creators are using it to generate SVG images.  At this point, it is next to impossible to ask up to a dozen software publishers to update their working SVG backends to accommodate one rendering program.

To put it another way, fixing this issue will help adopt SVG throughout the web.  Most content creators are not technically savvy enough to realize that this simple issue causes their fonts to render incorrectly.  They just see that it's broken.  Thus, they'll choose more stable image platforms such as PNG.

Isn't it a noble goal to increase SVG adoption on the web?  This goal is easily achievable by a fix that was already implemented!  I sincerely hope that the community can re-evaluate this issue and do better to address the repeated feedback given by its users.