Bug 206459

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 Flags
For the bots - missing test results
none
For the bots - missing test results
none
For the bots - missing test results
none
Patch and layout test
none
Patch and layout test
none
To land none

Daniel Bates
Reported 2020-01-17 16:38:20 PST
Add SPI to allow a client to show and hide the placeholder in a form control.
Attachments
For the bots - missing test results (10.78 KB, patch)
2020-01-17 16:39 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
For the bots - missing test results (10.78 KB, patch)
2020-01-17 16:42 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
For the bots - missing test results (10.77 KB, patch)
2020-01-17 16:43 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch and layout test (15.16 KB, patch)
2020-01-21 09:57 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch and layout test (19.74 KB, patch)
2020-01-21 10:51 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
To land (19.86 KB, patch)
2020-01-21 14:59 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-01-17 16:38:34 PST
<rdar://problem/58700534>
Daniel Bates
Comment 2 2020-01-17 16:39:57 PST
Created attachment 388112 [details] For the bots - missing test results
Daniel Bates
Comment 3 2020-01-17 16:42:21 PST
Created attachment 388114 [details] For the bots - missing test results
Daniel Bates
Comment 4 2020-01-17 16:43:28 PST
Created attachment 388115 [details] For the bots - missing test results
Daniel Bates
Comment 5 2020-01-21 09:57:44 PST
Created attachment 388313 [details] Patch and layout test
Daniel Bates
Comment 6 2020-01-21 10:51:54 PST
Created attachment 388321 [details] Patch and layout test Expose SPI in legacy WebKit as well for some legacy clients.
Wenson Hsieh
Comment 7 2020-01-21 11:03:36 PST
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?
Daniel Bates
Comment 8 2020-01-21 11:55:39 PST
Modern WebKit IPI! Not SPI and I don't need SPI either. I'll fix up the English.
Daniel Bates
Comment 9 2020-01-21 11:59:35 PST
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
Wenson Hsieh
Comment 10 2020-01-21 12:06:58 PST
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!
Daniel Bates
Comment 11 2020-01-21 14:59:27 PST
Created attachment 388356 [details] To land
Daniel Bates
Comment 12 2020-01-21 15:10:34 PST
Comment on attachment 388356 [details] To land Clearing flags on attachment: 388356 Committed r254886: <https://trac.webkit.org/changeset/254886>
Daniel Bates
Comment 13 2020-01-21 15:10:36 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.