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)
Tracking
()
RESOLVED
DUPLICATE
of bug 1375476
People
(Reporter: kerberos0818, Unassigned)
References
(Blocks 2 open bugs)
Details
(Keywords: regression, stale-bug, testcase)
Attachments
(4 files)
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
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox55:
--- → affected
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
Updated•7 years ago
|
Comment 4•7 years ago
|
||
Too late for firefox 52, mass-wontfix.
Comment 5•7 years ago
|
||
Updated•7 years ago
|
Summary: [e10s] font-size style for <option> elements → [e10s] font-size on <option> elements ignored
Updated•7 years ago
|
Summary: [e10s] font-size on <option> elements ignored → [e10s] <option> elements don't inherit <select> font-size
Comment 6•7 years ago
|
||
Too late for 53 since we are shipping next week. We could still possibly take a patch for 54 though.
Comment 7•7 years ago
|
||
Jet, this seems like an unfortunate regression, is there any chance we could get this fixed for 54?
Flags: needinfo?(bugs)
Comment 8•7 years ago
|
||
(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)
Comment 9•7 years ago
|
||
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)
Comment 10•7 years ago
|
||
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?
Comment 11•7 years ago
|
||
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.
Comment 12•7 years ago
|
||
Comment 13•7 years ago
|
||
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
Comment 14•7 years ago
|
||
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.
Comment 15•7 years ago
|
||
(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.
Comment 16•7 years ago
|
||
Any updates on this bug, Tommy? :)
Comment 17•7 years ago
|
||
Hi Ryan, I'll still fully support stylo in Q3. So, I didn't investigate this bug yet.
Flags: needinfo?(kuoe0)
Comment 18•7 years ago
|
||
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
Updated•7 years ago
|
Comment 19•7 years ago
|
||
I'll go back to work on this bug.
Updated•7 years ago
|
Updated•7 years ago
|
Priority: P1 → P3
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Updated•6 years ago
|
Status: ASSIGNED → NEW
Updated•6 years ago
|
Assignee: kuoe0.tw → nobody
Comment 24•5 years ago
|
||
Any plans to fix this?
Comment 25•5 years ago
|
||
I think this should work in nightly, can somebody confirm?
Comment 26•5 years ago
|
||
font-size is working in latest nightly. Any other font-* properties are not working (same behaviour in Google Chrome)
Comment 27•5 years ago
|
||
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:
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Comment 28•5 years ago
•
|
||
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.
Description
•