Summary: | [iOS][FCR] Add new picker for select elements | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aditya Keerthi <akeerthi> | ||||||
Component: | Forms | Assignee: | Aditya Keerthi <akeerthi> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, hi, thorton, webkit-bug-importer, wenson_hsieh | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | iPhone / iPad | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Aditya Keerthi
2021-01-29 13:52:37 PST
Created attachment 418768 [details]
Patch
Comment on attachment 418768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418768&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8027 > + // and for select elements. I feel like these comments are a bit silly :) (I also probably r+'d them) > Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm:473 > +@interface UIAction (IPI) > +- (void)_performActionWithSender:(id)sender; > +@end This should be in a SPI header, no? > Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm:476 > + WKContentView *_view; Should this be strong (or __weak)? What guarantees we don't outlive it (and maybe get something accidentally called on us?)? Even worse, dealloc indirectly calls things on _view, so even just something accidentally extending our lifetime would be disastrous (and there are no guarantees with refcounted objects). Created attachment 418867 [details]
Patch
(In reply to Tim Horton from comment #3) > Comment on attachment 418768 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=418768&action=review > > > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8027 > > + // and for select elements. > > I feel like these comments are a bit silly :) (I also probably r+'d them) Removed the comment. > > Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm:473 > > +@interface UIAction (IPI) > > +- (void)_performActionWithSender:(id)sender; > > +@end > > This should be in a SPI header, no? Moved to an SPI header. > > Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm:476 > > + WKContentView *_view; > > Should this be strong (or __weak)? What guarantees we don't outlive it (and > maybe get something accidentally called on us?)? > > Even worse, dealloc indirectly calls things on _view, so even just something > accidentally extending our lifetime would be disastrous (and there are no > guarantees with refcounted objects). Updated to use __weak. Committed r272334: <https://trac.webkit.org/changeset/272334> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418867 [details]. |