font-size-adjust no longer works

VERIFIED FIXED in Firefox 51

Status

()

Core
Graphics: Text
P1
normal
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: Gérard Talbot, Assigned: lsalzman)

Tracking

({regression, testcase})

51 Branch
mozilla53
All
Linux
regression, testcase
Points:
---

Firefox Tracking Flags

(firefox50 unaffected, firefox51 fixed, firefox52 fixed, firefox53 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

a year ago
Self-explanatory test
---------------------

http://test.csswg.org/suites/css-fonts-3_dev/nightly-unstable/html/font-size-adjust-003.htm


Notes
-----

- That test passes in Firefox 50.1 buildID=20161209094039
- but it fails in Firefox 53.0a1 buildID=20170112030301
- The fallback font "DejaVu Sans" (because the first 3 are not available and/or not installed) is chosen on my system but the font size adjustment does not seem to be executed
- I chose 'Graphics: Text' as Component but I am not sure what's the best component here
- I use Linux 3.13.0-107-generic x86_64, Qt: 4.8.6, KDE 4.14.13; Kubuntu (trusty) 14.04.5 LTS
- I've searched (quickly but not thoroughly) for duplicates and did not find any.
(Reporter)

Comment 1

a year ago
I am marking this as UNCONFIRMED for now. Can a bug triager check if this bug happens also under Windows and/or Mac OS X please? Thank you!
Status: NEW → UNCONFIRMED
Ever confirmed: false
Keywords: regression, testcase
(Assignee)

Updated

a year ago
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Ever confirmed: true
Priority: -- → P1
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Version: 53 Branch → 51 Branch
(Assignee)

Comment 2

a year ago
Created attachment 8826320 [details] [diff] [review]
make gfxFontconfigFont keep track of its actual adjusted size

It looks like every platform font list type EXCEPT fontconfig tracks the adjusted size of the font. Since fontconfig fonts only ever recorded the proper adjusted size in the Cairo font matrix, this causes problem when recreating the Skia font.

This patch makes fontconfig fonts behave like all the other fonts by actually tracking its adjusted size.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Attachment #8826320 - Flags: review?(jfkthame)
Comment on attachment 8826320 [details] [diff] [review]
make gfxFontconfigFont keep track of its actual adjusted size

Review of attachment 8826320 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxFcPlatformFontList.cpp
@@ +814,2 @@
>      gfxFont* newFont =
> +        new gfxFontconfigFont(scaledFont, renderPattern, adjustedSize, this, aFontStyle, aNeedsBold);

This line was too long previously, but it's even worse now ... please wrap.
Attachment #8826320 - Flags: review?(jfkthame) → review+

Comment 4

a year ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/456f50aee1e1
make gfxFontconfigFont keep track of its actual adjusted size. r=jfkthame

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/456f50aee1e1
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Comment 6

a year ago
Created attachment 8826571 [details] [diff] [review]
make gfxFontconfigFont keep track of its actual adjusted size

Fixed line-wrapping. Carrying r+ from :jfkthame.
Attachment #8826320 - Attachment is obsolete: true
Attachment #8826571 - Flags: review+
(Assignee)

Comment 7

a year ago
Comment on attachment 8826571 [details] [diff] [review]
make gfxFontconfigFont keep track of its actual adjusted size

Approval Request Comment
[Feature/Bug causing the regression]: bug 1278957, so 51+
[User impact if declined]: CSS font-size-adjust is broken on Linux
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: aurora, beta
[Is the change risky?]: No
[Why is the change risky/not risky?]: Only affects fonts on Linux, and only when font-size-adjust is used, and even then, only by modifying the size a bit.
[String changes made/needed]: None
Attachment #8826571 - Flags: approval-mozilla-beta?
Attachment #8826571 - Flags: approval-mozilla-aurora?
Comment on attachment 8826571 [details] [diff] [review]
make gfxFontconfigFont keep track of its actual adjusted size

regression fix for font-size-adjust on linux, aurora52+
Attachment #8826571 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

a year ago
status-firefox50: --- → unaffected
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox-esr45: --- → unaffected
(Reporter)

Comment 9

a year ago
The test now passes in Firefox 53.0a1 buildID=20170113030227 . Thank you Lee Salzman and Jonathan Kew for such diligent fix, resolution.

Marking as VERIFIED
Status: RESOLVED → VERIFIED
status-firefox50: unaffected → ---
status-firefox51: affected → ---
status-firefox52: affected → ---
status-firefox-esr45: unaffected → ---

Comment 10

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/4749ac0674d3
status-firefox52: --- → fixed
(Assignee)

Comment 11

a year ago
Liz, is it still possible to get this in before 51 moves to release? I'd like to make sure the Skia content transition goes smoothly.
Flags: needinfo?(lhenry)
Lee, yes! The 51 RC build is  next Monday. Thanks for the fix.
status-firefox50: --- → unaffected
status-firefox51: --- → affected
Flags: needinfo?(lhenry)
Comment on attachment 8826571 [details] [diff] [review]
make gfxFontconfigFont keep track of its actual adjusted size

Fix for regression from 51, let's uplift it for the RC build next Monday.
Attachment #8826571 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 14

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/9f142b02bdac
status-firefox51: affected → fixed
You need to log in before you can comment on or make changes to this bug.