| Summary: | Add Legacy WebKit SPI and WebKit IPI to show and hide placeholder | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||||||||
| Component: | WebCore Misc. | Assignee: | Daniel Bates <dbates> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | cdumez, esprehn+autocc, ews-watchlist, gyuyoung.kim, mifenton, webkit-bug-importer, wenson_hsieh | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Local Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Daniel Bates
2020-01-17 16:38:20 PST
Created attachment 388112 [details]
For the bots - missing test results
Created attachment 388114 [details]
For the bots - missing test results
Created attachment 388115 [details]
For the bots - missing test results
Created attachment 388313 [details]
Patch and layout test
Created attachment 388321 [details]
Patch and layout test
Expose SPI in legacy WebKit as well for some legacy clients.
Comment on attachment 388321 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=388321&action=review > Source/WebKit/ChangeLog:10 > + Add Modern WebKit SPI to allow a client to control whether the placeholder can be shown or > + not when a form control is empty. This is for aesthetics. It doesn't look like this patch actually adds "Modern WebKit SPI to allow a client to control whether the placeholder can be shown…". Was this intended for a followup? > Source/WebCore/html/HTMLTextFormControlElement.cpp:178 > + m_canShowPlaceholder = canShowPlaceholder; Nit - is it necessary to updatePlaceholderVisibility(); in the case where m_canShowPlaceholder didn't change? Modern WebKit IPI! Not SPI and I don't need SPI either. I'll fix up the English. Comment on attachment 388321 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=388321&action=review Thanks for the review! >> Source/WebKit/ChangeLog:10 >> + not when a form control is empty. This is for aesthetics. > > It doesn't look like this patch actually adds "Modern WebKit SPI to allow a client to control whether the placeholder can be shown…". Was this intended for a followup? IPI >> Source/WebCore/html/HTMLTextFormControlElement.cpp:178 >> + m_canShowPlaceholder = canShowPlaceholder; > > Nit - is it necessary to updatePlaceholderVisibility(); in the case where m_canShowPlaceholder didn't change? That function has the smarts... didn't feel a branch to avoid the call would speed things up much. So I didn't do it Comment on attachment 388321 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=388321&action=review >>> Source/WebCore/html/HTMLTextFormControlElement.cpp:178 >>> + m_canShowPlaceholder = canShowPlaceholder; >> >> Nit - is it necessary to updatePlaceholderVisibility(); in the case where m_canShowPlaceholder didn't change? > > That function has the smarts... didn't feel a branch to avoid the call would speed things up much. So I didn't do it Sounds good — thanks for clarifying! Created attachment 388356 [details]
To land
Comment on attachment 388356 [details] To land Clearing flags on attachment: 388356 Committed r254886: <https://trac.webkit.org/changeset/254886> All reviewed patches have been landed. Closing bug. |