Closed Bug 1350258 Opened 7 years ago Closed 5 years ago

[e10s] <option> elements don't inherit <select> font-size

Categories

(Core :: Layout: Form Controls, defect, P3)

52 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1375476
Tracking Status
firefox52 --- wontfix
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix

People

(Reporter: kerberos0818, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, stale-bug, testcase)

Attachments

(4 files)

Attached image fire-fox-bug.png
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170316213829

Steps to reproduce:

add css for Select menu.
on nothing firefox addon.



Actual results:

1.but not enabled select menu css.
and firefox addon nothing.

2.specific enabled firefox addon.
enabled select menu css, font size changed bigger.




<!-- sample minimal HTML. -->
<!-- sample HTML start -->
<!DOCTYPE html>
<html>
<head>
</head>
<body>
<select style="font-size:90px;">
<option style="" value="red">red</option>
<option style="" value="green">green</option>
<option style="" value="blue">blue</option>
</select>
</body>
</html>

<!-- sample HTML end -->




****** specific  firefox addon example ********
****** enabled select menu css addon example ********
Make Link 
https://addons.mozilla.org/ja/firefox/addon/make-link/?src=userprofile

Drag & DropZones
https://addons.mozilla.org/ja/firefox/addon/drag-dropzones/

Image Stopper
https://addons.mozilla.org/ja/firefox/addon/image-stopper/

Tab Mix Plus
https://addons.mozilla.org/ja/firefox/addon/tab-mix-plus/





Expected results:

nothing firefox addon 
and 
select menu css enabled.
bug 910022 seems to forget / ignore it.
Blocks: e10s-select
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout: Form Controls
Ever confirmed: true
Keywords: testcase
Product: Firefox → Core
Summary: firefox Addon is required for css activation on select menu → [e10s] font-size style for <option> elements
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Too late for firefox 52, mass-wontfix.
Attached file tc.html
Summary: [e10s] font-size style for <option> elements → [e10s] font-size on <option> elements ignored
Summary: [e10s] font-size on <option> elements ignored → [e10s] <option> elements don't inherit <select> font-size
Too late for 53 since we are shipping next week. We could still possibly take a patch for 54 though.
Jet, this seems like an unfortunate regression, is there any chance we could get this fixed for 54?
Flags: needinfo?(bugs)
(In reply to Nathan Froyd [:froydnj] from comment #7)
> Jet, this seems like an unfortunate regression
> is there any chance we could get this fixed for 54?

I don't know. There seems to be a number of regressions from e10s and <select> styling. There are performance concerns around pushing too many things over IPC to get 100% of the pre-e10s behavior back. Pushing over a font size doesn't sound like too much, but I don't know how much font data (e.g., for Web fonts) actually exists in the chrome process. 

jfkthame: any ideas on how to best fix this?
Flags: needinfo?(bugs) → needinfo?(jfkthame)
Presumably this issue applies to all the font-* properties (family, style, weight, etc), not just font-size.

If this were implemented at the C++ level in Gecko, I think it would be trivial to include a serialized nsFont record as part of the data we push to the chrome process to show a popup; that shouldn't be so much data that it materially impacts performance.

However, it looks like the implementation actually lives in toolkit JS-land (toolkit/modules/Select{Content,Parent}Helper.jsm) which I guess means we'd need to read the computed style of the items and add all the relevant font-* properties to the record, as was done for background and color in bug 910022. That seems likely to be substantially more expensive than if we were doing it at the C++ level by serializing and deserializing style structs, but I expect it'd still be OK.

Web fonts would remain an issue, as a webfont loaded by the child process won't be available in the parent. I believe we have the capability to fix that, based on the font serialization/deserialization stuff that Lee has been doing for printing via the chrome process, but it's possible that the amount of data we'd be pushing across might sometimes lead to a perceptible lag in popping up the menu. A large webfont (e.g. for CJK use) could be as much as a megabyte of data (although that's rare; most are in the range of a few tens of kilobytes, sometimes into the low hundreds). Also, I don't know if we currently have the ability to do this from front-end JS; we might need to add API to make that possible.

If we don't push the web font across, the result would be that the popup renders with whatever is next in the font-family list, which is the same behavior as the page itself would show if the webfont failed to load for any reason. That's probably reasonable as a first step, at least; as a followup, we could experiment with also serializing webfonts and see whether the performance is a problem.
Flags: needinfo?(jfkthame)
Something like this seems to basically work. Note that it doesn't currently pass all the font properties that might potentially be set (nor other things such as letter-spacing, text-transform, or text-decoration); I'm not sure how far it's worth taking this. Do we want to handle *every* CSS property that could be used to style each of the <select> items, or is that overkill?
I pushed a try job with the above patch, just to check if it'll disrupt any current tests.... so far (with a bunch still pending), looks OK: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7761e1ed7476469e671e361a25da99909e659552.
font-size change could be easily perceived by users and unfortunately it's not working in Firefox.(Chrome, Edge, Opera OK, Safari NG)

Per comment 9, all font-* properties are probably not working correctly foe option text. I'd love to see we can fix them in order of importance.
Assignee: nobody → kuoe0
Status: NEW → ASSIGNED
Priority: -- → P1
See Also: → 1363152
I'm on PTO for the next week, but if someone wants to take over the patch from comment 10 and move forward with it, that's fine with me.
(In reply to Jonathan Kew (:jfkthame) [PTO until 2017-06-19] from comment #14)
> I'm on PTO for the next week, but if someone wants to take over the patch
> from comment 10 and move forward with it, that's fine with me.

Thanks, Jonathan. Tommy will follow up the fix.
Any updates on this bug, Tommy? :)
Hi Ryan, I'll still fully support stylo in Q3. So, I didn't investigate this bug yet.
Flags: needinfo?(kuoe0)
This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.
Keywords: stale-bug
I'll go back to work on this bug.
Blocks: 1404329
Blocks: e10s-select-styling
No longer blocks: e10s-select
Priority: P1 → P3
Status: ASSIGNED → NEW
Assignee: kuoe0.tw → nobody

Any plans to fix this?

I think this should work in nightly, can somebody confirm?

font-size is working in latest nightly. Any other font-* properties are not working (same behaviour in Google Chrome)

Ok, this was fixed in bug 1375476.

To support more properties, you can add to this list, though some of them probably require more careful consideration:

https://searchfox.org/mozilla-central/rev/486b64e4bff86b7988af8c8b80845404ad197533/toolkit/actors/SelectChild.jsm#27

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE

I.e., please file a bug if you think there's something important missing, I intentionally kept that list short :)

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

Attachment

General

Creator:
Created:
Updated:
Size: