Closed Bug 1595620 Opened 5 years ago Closed 3 months ago

Considering aliasing -webkit-font-feature-settings to font-feature-settings for webcompat

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
129 Branch
Webcompat Priority P3
Tracking Status
firefox129 --- fixed

People

(Reporter: twisniewski, Assigned: twisniewski, Mentored)

Details

(Keywords: dev-doc-complete, good-first-bug, Whiteboard: [lang=rust])

Attachments

(2 files, 8 obsolete files)

249.32 KB, image/png
Details
48 bytes, text/x-phabricator-request
Details | Review

As discussed in this Reddit thread, Japanese Wikipedia pages are using -webkit-font-feature-settings instead of font-feature-settings, which is causing some characters to use the wrong font in Firefox. As such we ought to consider supporting the prefix as an alias for webcompat's sake.

Adding "webkit" to extra_prefixes here + a test should do, if the syntax is the same.

Mentor: emilio
Keywords: good-first-bug
Priority: -- → P3
Whiteboard: [lang=rust]

Hey, could I please work on this bug? I'm completely new to this so any tips would be appreciated.

Attached file Bug 1595620 (obsolete) —

Added "webkit" to extra_prefixes

Depends on D53325

Pushed by nbeleuzu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f07b485c29ad Add support for -webkit-font-feature-settings as an alias of font-feature-settings.r=emilio

I think the devtools static database just needs updating to get this to stick. Maybe run ./mach devtools-css-db?

Assignee: nobody → mozilla

Hey Emilio,
I think I need your help as a mentor for this. I created https://hg.mozilla.org/try/rev/48e258d13a58218dd9b690a8e09b49d398eb5e0b (basically Mark's patch with Sean's addition) and pushed it to try, but the when I run the build artifact it does not seem to have made a change. In DevTools, -webkit-font-feature-settings is marked as an Invalid property name. :( Could you please give me a pointer?

Flags: needinfo?(emilio)

That patch has a bunch of unrelated formatting changes. Please remove them?

But the patch seems to work as expected after I build and re-run ./mach devtools-css-db. I get a diff like:

+  "-webkit-font-feature-settings": {
+    "isInherited": true,
+    "subproperties": [
+      "font-feature-settings"
+    ],
+    "supports": [],
+    "values": [
+      "inherit",
+      "initial",
+      "normal",
+      "revert",
+      "unset"
+    ]
+  },
Flags: needinfo?(emilio)

Turns out I pushed to try with artifact build enabled, which is not what I wanted here...

Attachment #9109244 - Attachment is obsolete: true
Attachment #9109299 - Attachment is obsolete: true
Attachment #9109330 - Attachment is obsolete: true
Attachment #9109396 - Attachment is obsolete: true
Attachment #9138257 - Attachment is obsolete: true

Jens, is this ready to land?

Flags: needinfo?(mozilla)
Webcompat Priority: ? → P3

(In reply to Mike Taylor [:miketaylr] from comment #17)

Jens, is this ready to land?

Hey Mike, in Phabricator, Emilio explained that I'll need to write an Intent to Prototype where I haven't felt ready about at that time. However, I now think that I can do that. I'll write an Intent to Prototype and if nothing comes back, I'll rebase the patch on latest trunk. :)

Flags: needinfo?(mozilla)

Ah! Sorry, I missed that comment. :)

Good luck, looking forward to reading the intent (and am supportive of it!)

fyi, I sent the mail yesterday, but it seems it is stuck in the moderation queue as I had not been subscribed to the list at that time. I hope it will be approved in the next day(s).

Mike, do you know whether that list is still moderated? I could think that due to the recent changes it's not on deck anymore? Do you have any suggestion on how to proceed?

Flags: needinfo?(miket)
Attached image intent email

Hi Jens, it looks like it made it through. :)

Flags: needinfo?(miket)

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Assignee: mozilla → nobody

Hey jens,

There's a small change requested by emilio
https://phabricator.services.mozilla.com/D53357#1623134
and push it to the finish line.
Can you do it? Or do you want someone else to finish it?

Flags: needinfo?(mozilla)

Hey Karl,

due to time constraints, I unfortunately cannot fix this the current week, however I could hop on to this in the week.
Would that be okay or would you rather see this bug fixed asap by someone else?

Cheers,
Jens

Flags: needinfo?(mozilla)

next week will be great! Thanks.

Assignee: nobody → mozilla
Status: NEW → ASSIGNED

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Assignee: mozilla → nobody
Status: ASSIGNED → NEW
Severity: normal → S3

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.

Flags: needinfo?(kenned62)
Assignee: nobody → twisniewski
Status: NEW → ASSIGNED
Attachment #9109310 - Attachment is obsolete: true
Attachment #9138259 - Attachment is obsolete: true
Attachment #9110597 - Attachment is obsolete: true
Pushed by twisniewski@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f3680d74bc7a Add support for -webkit-font-feature-settings as an alias of font-feature-settings. r=emilio
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 129 Branch

Related issues and pull requests

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

Attachment

General

Creator:
Created:
Updated:
Size: