Remove special layout code for customizable select [chromium/src : main]
Joey Arhar (Gerrit)
Joey Arhar added 1 comment![]( "Open in Gerrit")
return MakeGarbageCollected<LayoutBlockFlow>(this);Hey tkent, do you know why this method is hard coded to return a LayoutFlexibleBox/LayoutBlockFlow, or the answers to my questions in the comments?
Related details
- Kent Tamura
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 1 comment![]( "Open in Gerrit")
I thought it would be a good idea to remove this code since we now have other ways of doing this in the UA stylesheet, but do you agree?
Kent Tamura (Gerrit)
Kent Tamura added 2 comments![]( "Open in Gerrit")
return MakeGarbageCollected<LayoutBlockFlow>(this);Hey tkent, do you know why this method is hard coded to return a LayoutFlexibleBox/LayoutBlockFlow, or the answers to my questions in the comments?
I think this avoids breaking <select>'s internal layout. Without this, it would be broken if a user specifies non-`flex` values to `display` property.
It would be possible to remove this hard code by `!important` in the UA stylesheet, or adding a flex container in the UA shadow tree.
I thought it would be a good idea to remove this code since we now have other ways of doing this in the UA stylesheet, but do you agree?
- Joey Arhar
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 3 comments![]( "Open in Gerrit")
Kent TamuraHey tkent, do you know why this method is hard coded to return a LayoutFlexibleBox/LayoutBlockFlow, or the answers to my questions in the comments?
I think this avoids breaking <select>'s internal layout. Without this, it would be broken if a user specifies non-`flex` values to `display` property.
It would be possible to remove this hard code by `!important` in the UA stylesheet, or adding a flex container in the UA shadow tree.
Thanks, I'm going to avoid changing the web-exposed computed value for now.
Kent TamuraI thought it would be a good idea to remove this code since we now have other ways of doing this in the UA stylesheet, but do you agree?
Of course it's ok to remove this if we have better ways.
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
AI Code Reviewer (Gerrit)
AI Code Reviewer added 1 comment![]( "Open in Gerrit")
void ChildrenChanged(const ChildrenChange& change) override;Blink Style Guide: Naming - May leave obvious parameter names out of function declarations. The parameter name 'change' can be omitted here as the type 'ChildrenChange' is self-descriptive.
***
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options: \
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason \
\
This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent). \
AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve. \
[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Related details
- Joey Arhar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 3 comments![]( "Open in Gerrit")
Note to self: link to this bug: https://issues.chromium.org/issues/389585531
Done
case CSSSelector::kPseudoSelectHasSlottedButton:It turns out that this is the reason that the accessibility test was failing, and I didn't figure it out for almost a year >_>
void ChildrenChanged(const ChildrenChange& change) override;Blink Style Guide: Naming - May leave obvious parameter names out of function declarations. The parameter name 'change' can be omitted here as the type 'ChildrenChange' is self-descriptive.
***_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options: \
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason \
\
This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent). \
AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve. \
[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |