181884 2018-01-19 14:51:36 -0800 Present WKFileUploadPanel on correct ViewController 2018-01-19 16:00:13 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 megan_gardner megan_gardner aestes joepeck thorton webkit-bug-importer wenson_hsieh oldest_to_newest 1391201 0 megan_gardner 2018-01-19 14:51:36 -0800 Present WKFileUploadPanel on correct ViewController 1391205 1 331793 megan_gardner 2018-01-19 14:54:50 -0800 Created attachment 331793 Patch 1391212 2 331793 joepeck 2018-01-19 15:03:39 -0800 Comment on attachment 331793 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331793&action=review Looks good to me. There are other places that use -[UIViewController _viewControllerForFullScreenPresentationFromView:]. For example: UIProcess/ios/forms/WKAirPlayRoutePicker.mm Should they change as well? > Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:462 > + NSLog(@"Failed to find a view controller to show form validation popover"); Accidental log. 1391213 3 thorton 2018-01-19 15:08:08 -0800 (In reply to Joseph Pecoraro from comment #2) > Comment on attachment 331793 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=331793&action=review > > Looks good to me. > > There are other places that use -[UIViewController > _viewControllerForFullScreenPresentationFromView:]. For example: > UIProcess/ios/forms/WKAirPlayRoutePicker.mm > > Should they change as well? Seems likely. > > Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:462 > > + NSLog(@"Failed to find a view controller to show form validation popover"); > > Accidental log. Humorously that’s in the other version of this function. 1391215 4 331793 thorton 2018-01-19 15:11:45 -0800 Comment on attachment 331793 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331793&action=review > Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:456 > +static UIViewController *fallbackViewController(UIView *view) I wonder if we should put this in the UIDelegate implementation if the client returns null, so we don’t have to put it everywhere. > Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:471 > + _presentationViewController = [self.delegate viewControllerForPresentingFileUploadPanel:self]; What if this returns nil? Can it? 1391227 5 331793 thorton 2018-01-19 15:27:15 -0800 Comment on attachment 331793 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331793&action=review >> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:456 >> +static UIViewController *fallbackViewController(UIView *view) > > I wonder if we should put this in the UIDelegate implementation if the client returns null, so we don’t have to put it everywhere. In a followup! >>> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:462 >>> + NSLog(@"Failed to find a view controller to show form validation popover"); >> >> Accidental log. > > Humorously that’s in the other version of this function. Maybe RELEASE_LOG instead >> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:471 >> + _presentationViewController = [self.delegate viewControllerForPresentingFileUploadPanel:self]; > > What if this returns nil? Can it? (I think you want if !presentationViewController instead of an else) 1391233 6 megan_gardner 2018-01-19 15:38:50 -0800 https://trac.webkit.org/changeset/227247/webkit 1391234 7 webkit-bug-importer 2018-01-19 15:39:14 -0800 <rdar://problem/36673774> 1391251 8 megan_gardner 2018-01-19 15:58:28 -0800 InRadar <rdar://problem/35114892> 1391253 9 megan_gardner 2018-01-19 16:00:13 -0800 <rdar://problem/35114892> 331793 2018-01-19 14:54:50 -0800 2018-01-19 15:27:15 -0800 Patch bug-181884-20180119145450.patch text/plain 4426 megan_gardner SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCShyZXZpc2lvbiAyMjcyNDApCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIzIEBACisyMDE4LTAxLTE5ICBNZWdhbiBHYXJk bmVyICA8bWVnYW5fZ2FyZG5lckBhcHBsZS5jb20+CisKKyAgICAgICAgUHJlc2VudCBXS0ZpbGVV cGxvYWRQYW5lbCBvbiBjb3JyZWN0IFZpZXdDb250cm9sbGVyCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODE4ODQKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzM1MTE0ODkyPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFdlIHNob3VsZCBub3QgYmUgZ2V0dGluZyB0aGUgbWFpbiB2aWV3IGNvbnRyb2xsZXIg ZGlyZWN0bHksIGFzIGRvbmUgaW4gCisgICAgICAgIFZhbGlkYXRpb25CdWJibGVJT1MuIFRoaXMg d2lsbCBrZWVwIHVzIGZyb20gcHJlc2VudGluZyBvbiB2aWV3IGNvbnRyb2xsZXJzIGFyZSAKKyAg ICAgICAgaW4gdGhlIG1pZHN0IG9mIGJlaW5nIGRpc21pc3NlZCwgZXRjLCBjYXVzaW5nIHVzIHRv IGdldCBpbnRvIHN0YXRlcyB3aGVyZSB3ZSBsb3NlCisgICAgICAgIHRoZSBhYmlsaXR5IHRvIHBy ZXNlbnQgdGhlIFdLRmlsZVVwbG9hZFBhbmVsLgorCisgICAgICAgICogVUlQcm9jZXNzL2lvcy9X S0NvbnRlbnRWaWV3SW50ZXJhY3Rpb24ubW06CisgICAgICAgICgtW1dLQ29udGVudFZpZXcgdmll d0NvbnRyb2xsZXJGb3JQcmVzZW50aW5nRmlsZVVwbG9hZFBhbmVsOl0pOgorICAgICAgICAqIFVJ UHJvY2Vzcy9pb3MvZm9ybXMvV0tGaWxlVXBsb2FkUGFuZWwuaDoKKyAgICAgICAgKiBVSVByb2Nl c3MvaW9zL2Zvcm1zL1dLRmlsZVVwbG9hZFBhbmVsLm1tOgorICAgICAgICAoZmFsbGJhY2tWaWV3 Q29udHJvbGxlcik6CisgICAgICAgICgtW1dLRmlsZVVwbG9hZFBhbmVsIF9wcmVzZW50RnVsbHNj cmVlblZpZXdDb250cm9sbGVyOmFuaW1hdGVkOl0pOgorCiAyMDE4LTAxLTE4ICBKYXNvbiBNYXJj ZWxsICA8am1hcmNlbGxAYXBwbGUuY29tPgogCiAgICAgICAgIENoZXJyeS1waWNrIHIyMjcxNDYu IHJkYXI6Ly9wcm9ibGVtLzM2NjI0MzE5CkluZGV4OiBTb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9p b3MvV0tDb250ZW50Vmlld0ludGVyYWN0aW9uLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRWaWV3SW50ZXJhY3Rpb24ubW0JKHJldmlzaW9uIDIy NzA3NykKKysrIFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRWaWV3SW50ZXJh Y3Rpb24ubW0JKHdvcmtpbmcgY29weSkKQEAgLTQwNjUsNiArNDA2NSw4IEBAIC0gKHZvaWQpX3No b3dSdW5PcGVuUGFuZWw6KEFQSTo6T3BlblBhbmUKICAgICBbX2ZpbGVVcGxvYWRQYW5lbCBwcmVz ZW50V2l0aFBhcmFtZXRlcnM6cGFyYW1ldGVycyByZXN1bHRMaXN0ZW5lcjpsaXN0ZW5lcl07CiB9 CiAKKyNwcmFnbWEgbWFyayAtIFdLRmlsZVVwbG9hZFBhbmVsRGVsZWdhdGUKKwogLSAodm9pZClm aWxlVXBsb2FkUGFuZWxEaWREaXNtaXNzOihXS0ZpbGVVcGxvYWRQYW5lbCAqKWZpbGVVcGxvYWRQ YW5lbAogewogICAgIEFTU0VSVChfZmlsZVVwbG9hZFBhbmVsLmdldCgpID09IGZpbGVVcGxvYWRQ YW5lbCk7CkBAIC00MDczLDYgKzQwNzUsMTEgQEAgLSAodm9pZClmaWxlVXBsb2FkUGFuZWxEaWRE aXNtaXNzOihXS0ZpbAogICAgIF9maWxlVXBsb2FkUGFuZWwgPSBuaWw7CiB9CiAKKy0gKFVJVmll d0NvbnRyb2xsZXIgKil2aWV3Q29udHJvbGxlckZvclByZXNlbnRpbmdGaWxlVXBsb2FkUGFuZWw6 KFdLRmlsZVVwbG9hZFBhbmVsICopZmlsZVVwbG9hZFBhbmVsCit7CisgICAgcmV0dXJuIF9wYWdl LT51aUNsaWVudCgpLnByZXNlbnRpbmdWaWV3Q29udHJvbGxlcigpOworfQorCiAjcHJhZ21hIG1h cmsgLSBVSVRleHRJbnB1dE11bHRpRG9jdW1lbnQKIAogLSAodm9pZClfcmVzdG9yZUZvY3VzV2l0 aFRva2VuOihpZCA8TlNDb3B5aW5nLCBOU1NlY3VyZUNvZGluZz4pdG9rZW4KSW5kZXg6IFNvdXJj ZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9mb3Jtcy9XS0ZpbGVVcGxvYWRQYW5lbC5oCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9mb3Jtcy9XS0ZpbGVVcGxvYWRQYW5l bC5oCShyZXZpc2lvbiAyMjcwNzcpCisrKyBTb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9pb3MvZm9y bXMvV0tGaWxlVXBsb2FkUGFuZWwuaAkod29ya2luZyBjb3B5KQpAQCAtNDgsNiArNDgsNyBAQCBj bGFzcyBXZWJPcGVuUGFuZWxSZXN1bHRMaXN0ZW5lclByb3h5OwogQHByb3RvY29sIFdLRmlsZVVw bG9hZFBhbmVsRGVsZWdhdGUgPE5TT2JqZWN0PgogQG9wdGlvbmFsCiAtICh2b2lkKWZpbGVVcGxv YWRQYW5lbERpZERpc21pc3M6KFdLRmlsZVVwbG9hZFBhbmVsICopZmlsZVVwbG9hZFBhbmVsOwor LSAoVUlWaWV3Q29udHJvbGxlciAqKXZpZXdDb250cm9sbGVyRm9yUHJlc2VudGluZ0ZpbGVVcGxv YWRQYW5lbDooV0tGaWxlVXBsb2FkUGFuZWwgKilmaWxlVXBsb2FkUGFuZWw7CiBAZW5kCiAKICNl bmRpZiAvLyBQTEFURk9STShJT1MpCkluZGV4OiBTb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9pb3Mv Zm9ybXMvV0tGaWxlVXBsb2FkUGFuZWwubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdC9V SVByb2Nlc3MvaW9zL2Zvcm1zL1dLRmlsZVVwbG9hZFBhbmVsLm1tCShyZXZpc2lvbiAyMjcwNzcp CisrKyBTb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9pb3MvZm9ybXMvV0tGaWxlVXBsb2FkUGFuZWwu bW0JKHdvcmtpbmcgY29weSkKQEAgLTQ1MiwxMSArNDUyLDI1IEBAIC0gKHZvaWQpX3ByZXNlbnRQ b3BvdmVyV2l0aENvbnRlbnRWaWV3Q28KICAgICBbX3ByZXNlbnRhdGlvblBvcG92ZXIgcHJlc2Vu dFBvcG92ZXJGcm9tUmVjdDpDR1JlY3RJbnRlZ3JhbChDR1JlY3RNYWtlKF9pbnRlcmFjdGlvblBv aW50LngsIF9pbnRlcmFjdGlvblBvaW50LnksIDEsIDEpKSBpblZpZXc6X3ZpZXcgcGVybWl0dGVk QXJyb3dEaXJlY3Rpb25zOlVJUG9wb3ZlckFycm93RGlyZWN0aW9uQW55IGFuaW1hdGVkOmFuaW1h dGVkXTsKIH0KIAorCitzdGF0aWMgVUlWaWV3Q29udHJvbGxlciAqZmFsbGJhY2tWaWV3Q29udHJv bGxlcihVSVZpZXcgKnZpZXcpCit7CisgICAgZm9yIChVSVZpZXcgKmN1cnJlbnRWaWV3ID0gdmll dzsgY3VycmVudFZpZXc7IGN1cnJlbnRWaWV3ID0gY3VycmVudFZpZXcuc3VwZXJ2aWV3KSB7Cisg ICAgICAgIGlmIChVSVZpZXdDb250cm9sbGVyICp2aWV3Q29udHJvbGxlciA9IFtVSVZpZXdDb250 cm9sbGVyIHZpZXdDb250cm9sbGVyRm9yVmlldzpjdXJyZW50Vmlld10pCisgICAgICAgICAgICBy ZXR1cm4gdmlld0NvbnRyb2xsZXI7CisgICAgfQorICAgIE5TTG9nKEAiRmFpbGVkIHRvIGZpbmQg YSB2aWV3IGNvbnRyb2xsZXIgdG8gc2hvdyBmb3JtIHZhbGlkYXRpb24gcG9wb3ZlciIpOworICAg IHJldHVybiBuaWw7Cit9CisKIC0gKHZvaWQpX3ByZXNlbnRGdWxsc2NyZWVuVmlld0NvbnRyb2xs ZXI6KFVJVmlld0NvbnRyb2xsZXIgKil2aWV3Q29udHJvbGxlciBhbmltYXRlZDooQk9PTClhbmlt YXRlZAogewogICAgIFtzZWxmIF9kaXNtaXNzRGlzcGxheUFuaW1hdGVkOmFuaW1hdGVkXTsKLQot ICAgIF9wcmVzZW50YXRpb25WaWV3Q29udHJvbGxlciA9IFtVSVZpZXdDb250cm9sbGVyIF92aWV3 Q29udHJvbGxlckZvckZ1bGxTY3JlZW5QcmVzZW50YXRpb25Gcm9tVmlldzpfdmlld107CisgICAg CisgICAgaWYgKFtzZWxmLmRlbGVnYXRlIHJlc3BvbmRzVG9TZWxlY3RvcjpAc2VsZWN0b3IoZmls ZVVwbG9hZFBhbmVsRGlkRGlzbWlzczopXSkKKyAgICAgICAgX3ByZXNlbnRhdGlvblZpZXdDb250 cm9sbGVyID0gW3NlbGYuZGVsZWdhdGUgdmlld0NvbnRyb2xsZXJGb3JQcmVzZW50aW5nRmlsZVVw bG9hZFBhbmVsOnNlbGZdOworICAgIGVsc2UKKyAgICAgICAgX3ByZXNlbnRhdGlvblZpZXdDb250 cm9sbGVyID0gZmFsbGJhY2tWaWV3Q29udHJvbGxlcihfdmlldyk7CiAgICAgW19wcmVzZW50YXRp b25WaWV3Q29udHJvbGxlciBwcmVzZW50Vmlld0NvbnRyb2xsZXI6dmlld0NvbnRyb2xsZXIgYW5p bWF0ZWQ6YW5pbWF0ZWQgY29tcGxldGlvbjpuaWxdOwogfQogCg==