Bug 165122 – [iOS] Tapping on an HTML validation bubble should dismiss it

• Component: DOM
• Status: RESOLVED FIXED

[Chris Dumez]

Tapping on an HTML validation bubble should dismiss it. It currently does nothing.

[Chris Dumez]

Created attachment 295656 [details]
WIP Patch

[Chris Dumez]

<rdar://problem/29429372>

[Chris Dumez]

Created attachment 295741 [details]
Patch

[Tim Horton]

Comment on attachment 295741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295741&action=review

> Source/WebCore/platform/ios/ValidationBubbleIOS.mm:52
> +- (WebValidationBubbleTapRecognizer *)initWithPopoverController:(UIViewController*)popoverController withPopoverView:(UIView *)popoverView

UIViewController star's on the wrong side. Also, do you really need to pass both? isn't popoverController.view popoverView?

[Chris Dumez]

(In reply to comment #4)
> Comment on attachment 295741 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295741&action=review
> 
> > Source/WebCore/platform/ios/ValidationBubbleIOS.mm:52
> > +- (WebValidationBubbleTapRecognizer *)initWithPopoverController:(UIViewController*)popoverController withPopoverView:(UIView *)popoverView
> 
> UIViewController star's on the wrong side. Also, do you really need to pass
> both? isn't popoverController.view popoverView?

Good point. I'll fix.

[Chris Dumez]

Created attachment 295761 [details]
Patch

[Chris Dumez]

(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 295741 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=295741&action=review
> > 
> > > Source/WebCore/platform/ios/ValidationBubbleIOS.mm:52
> > > +- (WebValidationBubbleTapRecognizer *)initWithPopoverController:(UIViewController*)popoverController withPopoverView:(UIView *)popoverView
> > 
> > UIViewController star's on the wrong side. Also, do you really need to pass
> > both? isn't popoverController.view popoverView?
> 
> Good point. I'll fix.

Fixed.

[Simon Fraser (smfr)]

Comment on attachment 295761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295761&action=review

> Source/WebCore/platform/ios/ValidationBubbleIOS.mm:58
> +    _tapGestureRecognizer = adoptNS([[getUITapGestureRecognizerClass() alloc] initWithTarget:self action:@selector(dismissPopover)]);

Are you sure this doesn't create a ref cycle?

[Chris Dumez]

Comment on attachment 295761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295761&action=review

>> Source/WebCore/platform/ios/ValidationBubbleIOS.mm:58
>> +    _tapGestureRecognizer = adoptNS([[getUITapGestureRecognizerClass() alloc] initWithTarget:self action:@selector(dismissPopover)]);
> 
> Are you sure this doesn't create a ref cycle?

I see WebValidationBubbleTapRecognizer's dealloc getting called so it seems fine.

[Chris Dumez]

Created attachment 295892 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 295892 [details]
Patch

Clearing flags on attachment: 295892

Committed r209253: <http://trac.webkit.org/changeset/209253>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.