Closed
Bug 1511570
Opened 6 years ago
Closed 6 years ago
content-language processing doesn't invalidate style data
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
VERIFIED
FIXED
mozilla65
People
(Reporter: Fanolian+BMO, Assigned: emilio)
References
(Blocks 1 open bug, )
Details
Attachments
(6 files)
(Fonts mentioned below apply to Windows only)
Prerequisite:
To reproduce this bug, I think it needs a few CJK fonts installed in the system. In this case Microsoft JhengHei, Meiryo, and MingLiU_HKSCS (or SimSun if you test it on other zh-CN sites) are used. I have all Microsoft provided fonts installed.
Steps to reproduce:
(Please refer to the attached screencast.)
1. Visit https://hk.entertainment.appledaily.com/entertainment/daily/article/20181201/20558431.
2. Change the main article font by adding a font-family:serif entry in Inspector.
** DO NOT assign a specific fontname. USE ONLY serif (or sans-serif). **
3. Observe the font change.
Acutal result:
Chinese characters change from JhengHei to Meiryo instead of MingLiU_HKSCS, the default font for zh-HK sites. It may not happen 100% of the time. If so, reload the page and retry a few times.
"Workaround":
The font will display/be chosen correctly after I go to Options > Fonts & Colors > Advanced, select another language, and press OK without making any changes.
This workaround fails when the page is reloaded, even if I permanently modifty the CSS with extensions like Stylus (which is how I first notice the bug).
Expected result:
Either the page follows the value of http-equiv="content-language" (just like pre-bug1438911), or assign at least a serif font defined in whatever language Nightly defaults to.
I understand that http-equiv="content-language" is not recommended. If the current display behavior is intended however, the "workaround" should not be working as it is confusing (though I assume no one will do this in daily browsing).
Note:
If I specify a font rather than serif or sans-serif, this bug is not exhibited.
According to Mozregression, the first bad Nightly is 2018-02-20 and regressed by bug 1438911.
This is a reduced testcase but it exhibits a slightly different behavior.
Upon opening the testcase the first time the font is wrong (Meiryo) most of the time. Reloading the page will have a chance to display correctly (MingLiU_HKSCS). The workaround in comment 0 still applies.
Before bug 1438911 is fixed, reloading this testcase will not randomly display correctly but only after opening devtool, or applying the workaround.
As a reference, this is a modified reduced testcase which declares language properly. It always displays with preferred proportional font for zh-HK.
Updated•6 years ago
|
Blocks: 1438911
Keywords: regression
Assignee | ||
Comment 4•6 years ago
|
||
If bug 1438911 changed the behavior here that means that there's a pre-existing bug / race somewhere else that was being hidden by the fact we deferred the charset change.
I'll try to take a look, I'll ask if I have trouble to repro.
Flags: needinfo?(emilio)
Assignee | ||
Comment 5•6 years ago
|
||
(In reply to Fanolian from comment #1)
> Before bug 1438911 is fixed, reloading this testcase will not randomly
> display correctly but only after opening devtool, or applying the workaround.
Ah, so if I'm reading this correctly that bug _improved_ the behavior, right?
That probably reinforces the hypothesis that there's something on our code that looks at the current charset but doesn't react to it changing properly. That bug makes the change happen sooner, which increases the chances of whatever the bogus code is of reading the right thing.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
> (In reply to Fanolian from comment #1)
> > Before bug 1438911 is fixed, reloading this testcase will not randomly
> > display correctly but only after opening devtool, or applying the workaround.
>
> Ah, so if I'm reading this correctly that bug _improved_ the behavior, right?
I would not say this is an improvement :). (consistently wrong -> randomly wrong)
Perhaps it is a side effect, after bug 1438911 I cannot apply font-family:serif/sans-serif to this site anymore with Stylus. It would not apply the expected font, as defined in Options, for me.
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Fanolian from comment #6)
> (In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
> > (In reply to Fanolian from comment #1)
> > > Before bug 1438911 is fixed, reloading this testcase will not randomly
> > > display correctly but only after opening devtool, or applying the workaround.
> >
> > Ah, so if I'm reading this correctly that bug _improved_ the behavior, right?
>
> I would not say this is an improvement :). (consistently wrong -> randomly
> wrong)
Well, it only is consistently wrong because there's no dynamic change to the page, so it's not really consistent either.
Assignee | ||
Comment 8•6 years ago
|
||
In my machine zooming into the page causes the second line to switch from the generic serif font on my system to noto serif.
I know why, though I think ideally the two lines should render the same? We don't seem to be using content-language to determine the default font-family, is that expected Jonathan?
Flags: needinfo?(jfkthame)
Assignee | ||
Comment 9•6 years ago
|
||
I'll write a patch, this is pretty broken.
Assignee: nobody → emilio
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: CJK font change via modifying CSS may not apply correctly for sites using http-equiv="content-language" → content-language processing doesn't invalidate style data
Comment 10•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)
> Created attachment 9029177 [details]
> Test-case showing the underlying bug.
>
> In my machine zooming into the page causes the second line to switch from
> the generic serif font on my system to noto serif.
>
> I know why, though I think ideally the two lines should render the same? We
> don't seem to be using content-language to determine the default
> font-family, is that expected Jonathan?
I can reproduce the issue since Firefox58(en-US) windows10 Home 64bit Japanese edition.
Clear cache
Load the test case(attachment 9029177 [details])
Times New Roman -> SimSun after zoom in/out -> Times New Roman after Reload(F5)
Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5f721a664bf64fed99184a866b60c24a6afcb3a0&tochange=62cebea1d1578461a423a9ca7848706455db9ea5
Regressed by: 7b3215e18443 J. Ryan Stinnett — Bug 1330412 - Enable Stylo by default
Assignee | ||
Comment 11•6 years ago
|
||
This is needed because content-language can affect the default
computed values for a given document.
Assignee | ||
Comment 12•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)
> Created attachment 9029177 [details]
> Test-case showing the underlying bug.
>
> In my machine zooming into the page causes the second line to switch from
> the generic serif font on my system to noto serif.
>
> I know why, though I think ideally the two lines should render the same? We
> don't seem to be using content-language to determine the default
> font-family, is that expected Jonathan?
I'm ~sure now that this is not expected, I think... I took a quick look and filed bug 1511675. All this font stuff is a huge mess.
Flags: needinfo?(emilio)
Reporter | ||
Comment 13•6 years ago
|
||
Is bug 1349852 related to this bug?
Assignee | ||
Comment 14•6 years ago
|
||
(In reply to Fanolian from comment #13)
> Is bug 1349852 related to this bug?
Yeah, to some extent.
Comment 15•6 years ago
|
||
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ddd8bdd2f05e
Make content-language invalidate style data. r=jfkthame
Comment 16•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Updated•6 years ago
|
Flags: qe-verify+
Comment 17•6 years ago
|
||
I was able to reproduce this bug on an affected Nightly build 65.0a1 (2019-01-21), and using the STR from comment 10.
The issue is no longer repro on 65.0 (20190121133710) running Windows 10 x64.
Updated•2 years ago
|
Flags: needinfo?(jfkthame)
You need to log in
before you can comment on or make changes to this bug.
Description
•