Bug 206459 - Add Legacy WebKit SPI and WebKit IPI to show and hide placeholder
Summary: Add Legacy WebKit SPI and WebKit IPI to show and hide placeholder
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-17 16:38 PST by Daniel Bates
Modified: 2020-01-21 15:10 PST (History)
7 users (show)

See Also:

Attachments
For the bots - missing test results (10.78 KB, patch)
2020-01-17 16:39 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
For the bots - missing test results (10.78 KB, patch)
2020-01-17 16:42 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
For the bots - missing test results (10.77 KB, patch)
2020-01-17 16:43 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
Patch and layout test (15.16 KB, patch)
2020-01-21 09:57 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
Patch and layout test (19.74 KB, patch)
2020-01-21 10:51 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
To land (19.86 KB, patch)
2020-01-21 14:59 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2020-01-17 16:38:20 PST 
Add SPI to allow a client to show and hide the placeholder in a form control.
Comment 1 Radar WebKit Bug Importer 2020-01-17 16:38:34 PST 
<rdar://problem/58700534>
Comment 2 Daniel Bates 2020-01-17 16:39:57 PST 
Created attachment 388112 [details]
For the bots - missing test results
Comment 3 Daniel Bates 2020-01-17 16:42:21 PST 
Created attachment 388114 [details]
For the bots - missing test results
Comment 4 Daniel Bates 2020-01-17 16:43:28 PST 
Created attachment 388115 [details]
For the bots - missing test results
Comment 5 Daniel Bates 2020-01-21 09:57:44 PST 
Created attachment 388313 [details]
Patch and layout test
Comment 6 Daniel Bates 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.
Comment 7 Wenson Hsieh 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?
Comment 8 Daniel Bates 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.
Comment 9 Daniel Bates 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
Comment 10 Wenson Hsieh 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!
Comment 11 Daniel Bates 2020-01-21 14:59:27 PST 
Created attachment 388356 [details]
To land
Comment 12 Daniel Bates 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>
Comment 13 Daniel Bates 2020-01-21 15:10:36 PST 
All reviewed patches have been landed.  Closing bug.