Closed Bug 935056 Opened 7 years ago Closed 3 months ago
In SVG, small font-sizes are not honored: "minimum font size" limit is enforced
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release) Build ID: 20131025151332 Steps to reproduce: Load the attached SVG file. This has font-size declared in style classes, from 5px through 35px. Actual results: There appears to be a minimum px size acceptable for font-size, somewhat larger than 10px. All sizes declared smaller than that are displayed in this minimum size. If the image is scaled (by varying the size of the window), this minimum size is also scaled, so it's not an absolute font size that is the limiting factor. Expected results: Text declared with smaller font-sizes than the de facto minimum should be displayed smaller. This used to work; it broke in the last week or so, probably in FF 25.
I don't fully understand the problem. I tested the attached SVG file in FF 24, 25 & 26 (On Win7). I also tested on Google Chrome. The provided file behaved identically on all browsers. Could you please provide some screen captures and a clearer explanation of what isn't working correctly?
I suspect you have a minimum font size set. SVG text should ignore that but no longer does.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Bug 839955 is very suspicious. Reporter, do you see the problem if you flipped "svg.text.css-frames.enabled" from about:config?
Yes, setting svg.text.css-frames.enabled to false makes the problem go away, and the file displays as should (and as it does in Chrome). I don't understand what this feature does (are about:config items documented somewhere?), but this behavior of it is really wrong-headed. In the test file (font-size.svg), the size of the smallest string is not all that small. Something is *interpreting* it as small because "5px" seems small, but the actual size that translates into depends entirely on the coordinate system of the SVG file. If someone wants a minimum text size, testing against the px value is the wrong way to do it -- the coord system in the test file leads to a "minimum font size" that is enormous. Text in an SVG file is a graphical object, and should (at least by default) preserve its relationship to other objects; otherwise labels will overwhelm the graphics (and other text strings) -- and the typical user (me) will have no idea what's causing the problem.
And yes, I do have a minimum text size set, to 12. I had completely forgotten about that. As I indicated above, the test file is being displayed with a minimum font size significantly larger than 12pt (the size I expected when I set the option) -- and as I argued above, enforcing any minimum size in SVG is probably the wrong answer.
Another wrinkle: I also have the option "Allow pages to set their own fonts" checked. In ordinary text this seems to override my minimum size of 12pt. It should also apply to SVG.
I take back some of that last comment: I can't be sure whether the minimum size is ignored in ordinary text. The minimum shown in ordinary text is much smaller than 12pt, but there does seem to be a minimum.
To confirm: * setting svg.text.css-frames.enabled to false displays the file correctly. * leaving svg.text.css-frames.enabled at true, but setting minimum font size to None in options also displays the file correctly, regardless of the "Allow pages to set fonts" option. * leaving svg.text.css-frames.enabled at true, setting a minimum font size to X interprets X as a user unit value in SVG, not a pt value, and enforces this minimum regardless of the "Allow pages to set fonts" option.
Thanks for confirming, R. Shelton. This is definitely a regression then with the new SVG text layout support.
Assignee: nobody → cam
Status: NEW → ASSIGNED
I *think* I'm safe in accessing aFont->mAllowZoom here when we're computing the font-size value, as indicated in the comment. Also, if you want me to rename mAllowZoom / -x-text-zoom to something that also encompasses whether minimum font sizes apply, I can do that.
Attachment #828364 - Flags: review?(dbaron)
Reproduced with 2013-11-13-03-02-05-mozilla-central-firefox-28.0a1.en-US.linux-x86_64.
OS: Windows XP → All
Summary: In SVG, small font-sizes are not honored (recently broken, in FF 25?) → In SVG, small font-sizes are not honored: "minimum font size" limit is enforced (recently broken, in FF 25?)
Probably not a serious enough bug to warrant landing on Release, but setting tracking? all the same.
Comment on attachment 828364 [details] [diff] [review] patch r=dbaron. (Though I'm not sure what the comment is suggesting might not be safe, but is safe in this case.)
Attachment #828364 - Flags: review?(dbaron) → review+
The comment is (trying to) say that aFont->mAllowZoom will always have a valid value, since if aRuleData->ValueForTextZoom()->GetUnit() == eCSSUnit_Null, we will have started with an inherited struct, and in all other cases we assign to mAllowZoom.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Regression from 25, we can take this as an uplift to branches if low risk fix seeing as we're quite late in FF26 beta please nominate asap for landing this week.
Comment on attachment 828364 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 839955 User impact if declined: text in SVG documents shown at incorrect size if a minimum font size is set Testing completed (on m-c, etc.): manual testing, plus an automated test; landed on m-c yesterday Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none
Backed out for maybe causing bug 939980: https://hg.mozilla.org/integration/mozilla-inbound/rev/ed622e651e1e It's an intermittent orange that triggers about four times a day, so we should know soon enough whether it did. I'll re-land it, and on beta/aurora, if it turns out that it didn't cause the intermittent orange.
The intermittent failures have stopped with the backout, so I won't land this on beta/aurora.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
We can wait for this to ride the trains, untracking.
Cameron, have you got any idea what's going on here? We need to fix this regression somehow.
I am not sure. Landing a slight tweak of this patch that always initializes allowZoom, and uses it instead of font->mAllowZoom: https://hg.mozilla.org/integration/mozilla-inbound/rev/29e6539961c3
I remain stumped by the intermittent failures, but pushing a try run just now on inbound's tip I couldn't reproduce the failure with a bunch of retriggers. So, relanding to see if it sticks again: https://hg.mozilla.org/integration/mozilla-inbound/rev/3425d772b2ba
https://tbpl.mozilla.org/php/getParsedLog.php?id=34066411&tree=Mozilla-Inbound Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/88ae89969aad
Target Milestone: mozilla28 → ---
And no, that wasn't the only one.
Since this has been around now since FF25 without causing any obvious amounts of user pain it's not really looking like something we need to be tracking as a potential release-blocking bug but if there's a low risk fix ready in the next couple of weeks we can consider an uplift nomination to Aurora/Beta.
This bug still exists in current versions of Firefox. Could the bug be updated to tell it affects current versions of Firefox?
If it's not in the state FIXED then that's sign enough that it affects current versions of Firefox. No updates necessary.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/8eef199bbda9 Don't apply minimum font sizes to SVG text. r=heycam
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/afdf4d60166d Don't apply minimum font sizes to SVG text. r=heycam
Flags: needinfo?(rmaries) → needinfo?(longsonr)
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/256602905ce8 Don't apply minimum font sizes to SVG text. r=heycam
Summary: In SVG, small font-sizes are not honored: "minimum font size" limit is enforced (recently broken, in FF 25?) → In SVG, small font-sizes are not honored: "minimum font size" limit is enforced
You need to log in before you can comment on or make changes to this bug.