Last Comment Bug 739804 - ⇓ character entity and similar symbols cause excessive line-spacing under GDI
: ⇓ character entity and similar symbols cause excessive line-spacing unde...
Status: VERIFIED FIXED
: regression
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla14
Assigned To: John Daggett (:jtd)
:
Mentors:
data:text/html,<div style="border:sol...
: 762601 764093 766132 (view as bug list)
Depends on: 759042 598900
Blocks: 705594
  Show dependency treegraph
 
Reported: 2012-03-27 15:12 PDT by Matthew Kidd
Modified: 2012-07-30 07:29 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
verified


Attachments
Partial screenshot illustrating problem with "1.3.0 and &dArr;" highlighted (39.35 KB, image/png)
2012-03-28 13:49 PDT, Matthew Kidd
no flags Details
content of about:support (46.53 KB, text/plain)
2012-03-28 14:21 PDT, Matthew Kidd
no flags Details
Small appearance of &#9733; (solid star) entity in Firefox 11 (30.44 KB, image/png)
2012-03-29 10:38 PDT, Matthew Kidd
no flags Details
Normal appearance of &#9733; (solid star) entity in Firefox 14 (30.62 KB, image/png)
2012-03-29 12:52 PDT, Matthew Kidd
no flags Details
screenshot (comparison before/after landing of Bug 705594) (69.57 KB, image/png)
2012-04-15 18:32 PDT, Alice0775 White
no flags Details
testcase, fallback for the down arrow character (1.78 KB, text/html)
2012-04-16 00:39 PDT, John Daggett (:jtd)
no flags Details
cambria math rendering, GDI vs. DirectWrite (19.12 KB, image/png)
2012-04-16 01:00 PDT, John Daggett (:jtd)
no flags Details
testcase, show all possible matches for the down arrow (1.79 KB, text/html)
2012-05-24 20:03 PDT, John Daggett (:jtd)
no flags Details
patch, don't use Cambria Math for symbol fallback (2.85 KB, patch)
2012-05-24 20:18 PDT, John Daggett (:jtd)
jfkthame: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
patch, fix the reftests that depended upon previous fallback behavior (1.50 KB, patch)
2012-06-13 23:34 PDT, John Daggett (:jtd)
smontagu: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
FF 14b10 vs Nightly 2012-03-09 (242.54 KB, image/jpeg)
2012-07-03 06:13 PDT, Paul Silaghi, QA [:pauly]
no flags Details

Description Matthew Kidd 2012-03-27 15:12:20 PDT
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120327 Firefox/14.0a1
Build ID: 20120327031149

Steps to reproduce:

Viewed Version History table on http://lajollabridge.com/Software/ACBLmerge/ACBLmergeAbout.htm


Actual results:

Left column (version numbers) <td> contents do not appear aligned to the top of the cell despite CSS specifying such. Actually the CSS seems to be handled correctly but when the downward arrow (&dArr;) entity is highlighted, its bounding box is seen to extend far above and below the glyph, leading to the apparent alignment issue.


Expected results:

&dArr; should have correct bounding box (as it does in Firefox 11) which then results in the correct rendering.
Comment 1 Tim (fmdeveloper) 2012-03-28 13:43:09 PDT
Don't see the issue, or don't understand the issue, with Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120328 Firefox/14.0a1 ID:20120328031211

Can you attach a screenshot highlighting the issue?
Comment 2 Matthew Kidd 2012-03-28 13:49:19 PDT
Created attachment 610287 [details]
Partial screenshot illustrating problem with "1.3.0 and &dArr;" highlighted
Comment 3 Tim (fmdeveloper) 2012-03-28 13:52:40 PDT
Thanks for the quick reply with the screenshot Matthew.

WFM - Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120328 Firefox/14.0a1 ID:20120328031211

Does the issue still occur if you start Firefox in Safe Mode? http://support.mozilla.com/en-US/kb/Safe+Mode

How about with a new, empty profile? http://support.mozilla.com/en-US/kb/Basic%20Troubleshooting#w_8-make-a-new-profile
Comment 4 Matthew Kidd 2012-03-28 14:08:37 PDT
Yes, both in Safe Mode and with a new profile.
Comment 5 Tim (fmdeveloper) 2012-03-28 14:17:00 PDT
Please post the contents of about:support as a text attachment to this bug
Comment 6 Matthew Kidd 2012-03-28 14:21:16 PDT
Created attachment 610295 [details]
content of about:support
Comment 7 Matthew Kidd 2012-03-29 10:37:15 PDT
I have observed another difference between Firefox 11 and Firefox 14 that may have some relevance for the current issue. The solid star (&#9733;) character entity seems too small in Firefox 11 relative to other character sizes. In Firefox 14 it seems to have the "right" size and incidentally a size that is identical to or at least very close to the size it has in IE8 and late versions of Chrome. I wonder if the change for that had some indirect impact on the &darr; bounding box.

Here is a URL that shows the star size issue:
http://www.lajollabridge.com/Events/2012/120311A.htm

Look for the part that says "* * * Masterpoint Winners * * *" near the top of the page. I'll attach a screenshot that shows it too.
Comment 8 Matthew Kidd 2012-03-29 10:38:39 PDT
Created attachment 610591 [details]
Small appearance of &#9733; (solid star) entity in Firefox 11
Comment 9 Matthew Kidd 2012-03-29 12:52:59 PDT
Created attachment 610653 [details]
Normal appearance of &#9733; (solid star) entity in Firefox 14
Comment 10 Nickolay_Ponomarev 2012-04-02 15:29:14 PDT
I too can reproduce on a WinXP nightly (although not on Mac) with the page from comment 0 or this:

data:text/html,<div style="border:solid green 1px"><span style="font-size: 16px; border: solid red 1px">&dArr;</span></div>

(There's a much larger vertical space between the <span>'s border and the <div>'s on the nightly.)

I probably won't get to it, but finding a regression window would help with figuring this problem out. See https://quality.mozilla.org/docs/bugzilla/guide-to-triaging-bugs-for-firefox/finding-a-regression-window/
Comment 11 Matthew Kidd 2012-04-02 15:41:29 PDT
I can try to narrow the regression date down though I might not have time this week. I am pretty sure the I did not see the problem in Firefox 13 (when it was Nightly) and I think I saw the problem in Firefox 14 about a week before reporting the bug, deciding at that point that it wasn't one of those little Nightly glitches that gets cleaned up a few days later.
Comment 12 Alex Keybl [:akeybl] 2012-04-09 15:20:36 PDT
Cautiously marking this as tracked for FF14 since there could be unexpected web regressions caused by this bug.
Comment 13 Jonathan Kew (:jfkthame) 2012-04-10 03:50:30 PDT
I'm guessing this is a result of the font fallback optimizations in bug 705594, resulting in a STIX font with huge ascent/descent getting used for the &dArr; character, where previously a different font was being found.
Comment 14 Jonathan Kew (:jfkthame) 2012-04-10 03:51:28 PDT
Errr, I didn't mean a STIX font; it's most likely Cambria Math that's being chosen.
Comment 15 Alice0775 White 2012-04-15 18:32:48 PDT
Created attachment 615219 [details]
screenshot (comparison before/after landing of Bug 705594)

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/85ba09eada58
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120308 Firefox/13.0a1 ID:20120308231132
Bad:
http://hg.mozilla.org/mozilla-central/rev/ead9016b4102
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120309 Firefox/13.0a1 ID:20120309043532
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=85ba09eada58&tochange=ead9016b4102


Regression window(m-c)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/63dce6d751c6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120308 Firefox/13.0a1 ID:20120308171833
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d5483dc18285
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120308 Firefox/13.0a1 ID:20120308180833
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=63dce6d751c6&tochange=d5483dc18285
Comment 16 John Daggett (:jtd) 2012-04-16 00:39:57 PDT
Created attachment 615264 [details]
testcase, fallback for the down arrow character

This specifies the list of fallback fonts that were added in bug 705594.  See fallback code here:

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxWindowsPlatform.cpp#921
Comment 17 John Daggett (:jtd) 2012-04-16 01:00:05 PDT
Created attachment 615267 [details]
cambria math rendering, GDI vs. DirectWrite

Testcase rendering with GDI and DirectWrite rendering
Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20120415 Firefox/14.0a1
with and w/o DirectWrite enabled

There have been past bugs related to the metrics used for Cambria Math, this appears to be related to that.
Comment 18 Jonathan Kew (:jfkthame) 2012-04-16 01:17:08 PDT
I guess you're thinking of bug 598900, among others, which remains an open issue.

The font fallback changes appear as a regression here because they're causing Cambria Math to be used on pages where it wasn't previously occurring.
Comment 19 John Daggett (:jtd) 2012-04-16 01:38:50 PDT
(In reply to Jonathan Kew (:jfkthame) from comment #18)
> The font fallback changes appear as a regression here because they're
> causing Cambria Math to be used on pages where it wasn't previously
> occurring.

I'm not arguing it's not a regression.  The question is whether to yank Cambria Math out of the list of hard-coded fonts (at least for the GDI case) or to fix the underlying problem with the metrics.
Comment 20 Jonathan Kew (:jfkthame) 2012-04-16 02:01:40 PDT
Well... I do think we should fix the underlying metrics problem, which manifests itself in a number of ways (we end up with surprisingly different metrics across platforms for the same font), but that's a more extensive issue and will almost certainly cause a bunch of fragile test breakage, etc., that we'll have to look at carefully.

If we want a quick/safe fix here to reduce the immediate pain, adjusting the hard-coded font list is probably the way to go.

(I still wish the fallback list for each platform was a pref rather than compiled-in....)
Comment 21 John Daggett (:jtd) 2012-05-24 20:03:10 PDT
Created attachment 627075 [details]
testcase, show all possible matches for the down arrow

Shows default rendering at font-size: 100px along with rendering for all Windows 7 fonts that contain a glyph for the down arrow.
Comment 22 John Daggett (:jtd) 2012-05-24 20:12:38 PDT
The "bug" here has to do with the handling of Cambria Math in GDI-mode only (i.e. XP or Win7 with old drivers).  Under Windows 7 normal DirectWrite rendering of Cambria Math is fine.  It will be dependent upon the environment to some degree, Segoe UI Symbols cannot be installed but Cambria Math must be installed.

The fact that the default font for this character changed is not in itself a "bug", only the handling of Cambria Math under GDI (note that Chrome shows the same problem for Cambria Math).

Steps to hopefully reproduce:

1. Run in GDI mode (if on Win7/Vista, explicitly enable 'gfx.direct2d.disable')
2. Load the "show all possible..." testcase

Result: default case uses Cambria Math (line height should match that of the Cambria Math line).

Expected result: the fallback case displays without the huge line height.
Comment 23 John Daggett (:jtd) 2012-05-24 20:18:09 PDT
Created attachment 627076 [details] [diff] [review]
patch, don't use Cambria Math for symbol fallback

Take out Cambria Math from fallback list covering symbols, lower its priority for non-BMP characters, add in Lucida Sans Unicode for symbol fallback.
Comment 24 Jonathan Kew (:jfkthame) 2012-05-26 02:58:30 PDT
Comment on attachment 627076 [details] [diff] [review]
patch, don't use Cambria Math for symbol fallback

Fine, I guess. (I still wish these fallback lists were loaded from prefs rather than compiled in to the code, though....)
Comment 25 John Daggett (:jtd) 2012-05-27 20:24:54 PDT
(In reply to Jonathan Kew (:jfkthame) from comment #24)
> Fine, I guess. (I still wish these fallback lists were loaded from
> prefs rather than compiled in to the code, though....)

The hardcoded fontlists are there to try and speed the system fallback
process by checking specific platform-specific fonts for certain
ranges, based on testing to determine when system fallback occurs
given default fonts on each platform.  The goal is to try a few select
fonts to eliminate the need to do a global search, either via scanning
all cmaps or via a platform-supported method.  As such it's not really
a "preference", nor is it a simple list to derive, since it's based on
analyzing when *system* fallback occurs (i.e. beyond the set of fonts
in the prefs list).

It might make sense to have a single, global fallback list for those
who desire to tweak but I think exposing a slew of range-specific
fallback lists is not a great idea.
Comment 26 John Daggett (:jtd) 2012-05-27 20:31:58 PDT
Pushed to inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/36f327aaffa5
Comment 27 Phil Ringnalda (:philor) 2012-05-27 22:16:54 PDT
reftests/bidi/729047-1.html and reftests/bugs/427730-1.html say they'd rather you didn't do that without consulting them. Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/c29e42966d59
Comment 28 Jonathan Kew (:jfkthame) 2012-05-27 23:38:24 PDT
Looks like the addition of Lucida Sans Unicode to the fallbacks for the symbol ranges is causing a change in the rendering of U+200B (zero width space), such that it affects the line height.
Comment 29 John Daggett (:jtd) 2012-05-27 23:55:16 PDT
Both of these tests are making assumptions about the line-height being the same indpendent of the default font.  Both these tests fail on a WinXP ja-jp locale with Nightly, they need an explicit line-height setting.  A lot of these bidi tests suffer from locale dependency, I guess I need to bite the bullet and fix these, grumble, grumble.
Comment 30 Jonathan Kew (:jfkthame) 2012-06-13 00:34:36 PDT
*** Bug 762601 has been marked as a duplicate of this bug. ***
Comment 31 Jonathan Kew (:jfkthame) 2012-06-13 00:34:41 PDT
*** Bug 764093 has been marked as a duplicate of this bug. ***
Comment 32 Alex Keybl [:akeybl] 2012-06-13 13:49:08 PDT
Is this patch low risk enough to consider for aurora/beta approval?
Comment 33 John Daggett (:jtd) 2012-06-13 23:34:26 PDT
Created attachment 633044 [details] [diff] [review]
patch, fix the reftests that depended upon previous fallback behavior

Set the line height on these tests so that the difference in characters used between the test and ref don't affect the linebox placement.
Comment 35 John Daggett (:jtd) 2012-06-13 23:46:55 PDT
(In reply to Alex Keybl [:akeybl] from comment #32)
> Is this patch low risk enough to consider for aurora/beta approval?

Yes, assuming the patch sticks this time, we should definitely try to get this into aurora/beta, it's a minor tweak that fixes an annoyance for those viewing pages with these characters on XP.
Comment 37 Alex Keybl [:akeybl] 2012-06-20 10:01:31 PDT
Please nominate for Aurora/Beta when ready, along with a risk/reward evaluation.
Comment 38 John Daggett (:jtd) 2012-06-25 17:50:31 PDT
Comment on attachment 627076 [details] [diff] [review]
patch, don't use Cambria Math for symbol fallback

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 705594
User impact if declined: line spacing problems for certain symbols in the Windows GDI case (XP/Win7 w/o hw accel)
Testing completed (on m-c, etc.): landed couple weeks ago
Risk to taking this patch (and alternatives if risky): Very low, this is really unlikely to have a negative impact.
String or UUID changes made by this patch: None
Note: the reftest changes are required to workaround font dependencies in the reftests
Comment 39 John Daggett (:jtd) 2012-06-25 17:51:05 PDT
Comment on attachment 633044 [details] [diff] [review]
patch, fix the reftests that depended upon previous fallback behavior

[Approval Request Comment]
-- see above --
Comment 40 Alex Keybl [:akeybl] 2012-06-26 09:46:44 PDT
Comment on attachment 627076 [details] [diff] [review]
patch, don't use Cambria Math for symbol fallback

[Triage Comment]
New regression in FF13, with a low risk fix. Let's get this on Aurora 15 and Beta 14.
Comment 42 Jonathan Kew (:jfkthame) 2012-07-03 05:48:44 PDT
*** Bug 766132 has been marked as a duplicate of this bug. ***
Comment 43 Paul Silaghi, QA [:pauly] 2012-07-03 06:13:54 PDT
Created attachment 638683 [details]
FF 14b10 vs Nightly 2012-03-09

I've tested this comparing nightly 2012-03-09 with FF 14b10 on Win XP, see the screenshot attached. Is this fixed ? I might not understand the issue, with test cases from comment 0 and comment 7 I don't see any differences.
Comment 44 Jonathan Kew (:jfkthame) 2012-07-03 06:21:50 PDT
Does that machine exhibit the issue in FF13? I'm guessing you don't have Cambria Math installed on that XP machine, and so the problem wouldn't have shown up there.
Comment 45 Paul Silaghi, QA [:pauly] 2012-07-10 05:34:09 PDT
Thanks Jonathan. I'm able to see the issue now on FF 13.0.1 loading the test cases from comment 0 and comment 21. 
Verified fixed on Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20100101 Firefox/14.0b11.
Comment 46 Paul Silaghi, QA [:pauly] 2012-07-30 07:29:04 PDT
Verified fixed on Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/20100101 Firefox/15.0b2.

Note You need to log in before you can comment on or make changes to this bug.