Bug 180756

Summary: REGRESSION(225597): Can't select a text box or web view on a page when VO is on.
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebCore Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch bfulgham: review+

Description Per Arne Vollan 2017-12-13 10:22:16 PST
We should use the NSApplication run loop until we can initialize accessibility without it.
Comment 1 Per Arne Vollan 2017-12-13 10:22:53 PST
<rdar://problem/35996158>
Comment 2 Per Arne Vollan 2017-12-13 10:30:52 PST
Created attachment 329231 [details]
Patch
Comment 3 Brent Fulgham 2017-12-13 11:06:37 PST
Comment on attachment 329231 [details]
Patch

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

Looks good. Please address the confusing comment. r=me assuming EWS is fine.

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=180756

<rdar://problem/35996158>

> Source/WebKit/Platform/IPC/mac/ConnectionMac.mm:593
> +#endif

I wonder if we really need to exclude this. Surely we want to avoid calling into _AXUIElementNotify... if NSApp is not running?

> Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:80
> +    // Leak a boost onto the WebContent process if the NSApplication run loop is not used.

I don't know what Leak a boost" means?
Comment 4 Per Arne Vollan 2017-12-13 12:21:50 PST
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 329231 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329231&action=review
> 
> Looks good. Please address the confusing comment. r=me assuming EWS is fine.
> 
> > Source/WebCore/ChangeLog:4
> > +        https://bugs.webkit.org/show_bug.cgi?id=180756
> 
> <rdar://problem/35996158>
> 
> > Source/WebKit/Platform/IPC/mac/ConnectionMac.mm:593
> > +#endif
> 
> I wonder if we really need to exclude this. Surely we want to avoid calling
> into _AXUIElementNotify... if NSApp is not running?
> 

I excluded this in case it has some unknown performance implications.

> > Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:80
> > +    // Leak a boost onto the WebContent process if the NSApplication run loop is not used.
> 
> I don't know what Leak a boost" means?

I will change the comment before landing. Thanks for reviewing!
Comment 5 Per Arne Vollan 2017-12-13 12:23:17 PST
Committed r225863: <https://trac.webkit.org/changeset/225863/webkit>
Comment 6 Simon Fraser (smfr) 2017-12-13 12:34:45 PST
Comment on attachment 329231 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).
> +
> +        No new tests.

Why is there no explanatory text here? What did you change? Why?
Comment 7 Per Arne Vollan 2017-12-14 08:33:23 PST
(In reply to Simon Fraser (smfr) from comment #6)
> Comment on attachment 329231 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329231&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Reviewed by NOBODY (OOPS!).
> > +
> > +        No new tests.
> 
> Why is there no explanatory text here? What did you change? Why?

This patch basically reverts https://trac.webkit.org/changeset/225597/webkit, and goes back to using the NSApplication run loop, since using the NSRunLoop broke VoiceOver functionality. We will attempt to use the NSRunLoop again, when we have a way to initialize accessibility in the WebContent process.