bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Alan Third]

On Mon, Apr 30, 2018 at 07:46:55PM -0400, Fu Yuan wrote:
> 
> * Symptom
> 
> On macOS, the AXSubrole of a childframe is set to AXStandardWindow. This
> setting causes other applications on the same machine to possible
> interfere Emacs. For example, a windows manager will reposition the
> childframe, as descussed here:
> https://github.com/emacs-lsp/lsp-ui/issues/107#issuecomment-385377352
> 
> In the discussion, koekeishiya suggested to set the attribute to
> kAXDialogSubrole or kAXFloatingWindowSubrole.

Unfortunately those are Carbon attributes and I don’t think we can (or
want to) set them directly.

It looks like the nearest equivalent is the NSWindow level attribute,
which we’re using for z‐groups.

Can you please try the following:

    (let ((frame (selected-frame)))
      (make-frame-command)
      (set-frame-parameter nil 'parent-frame frame)
      (set-frame-parameter nil 'z-group 'above))

and see what the UIElementInspector returns? (I can’t build it as I
don’t have xcode installed.)

If you’re able you could try applying this patch:

1 file changed, 1 insertion(+), 1 deletion(-)
src/nsterm.m | 2 +-

modified   src/nsterm.m
@@ -2028,7 +2028,7 @@ so some key presses (TAB) are swallowed by the system. */
     }
   else if (EQ (new_value, Qabove))
     {
-      window.level = NSNormalWindowLevel + 1;
+      window.level = NSFloatingWindowLevel;
       FRAME_Z_GROUP (f) = z_group_above;
     }
   else if (EQ (new_value, Qabove_suspended))

and run the lisp above again and see if it’s different.

-- 
Alan Third

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

Please keep the bug tracker Cc’d in.

On Fri, May 11, 2018 at 09:31:14PM +0800, Fu Yuan wrote:
> I first ran the lisp code and nothing changed. Then I patched source
> file and compiled, then ran the lisp code on the compiled emacs.
> Nothing different on that Emacs. The UIElementInspecter shows
> AXStandardWindow. Of course both time I used “emacs" command with
> “-Q" flag.
> 
> Is there anything else I can do?

OK, I looked into this a bit further and it seems this stuff is in the
accessibility API. I’ve attached a couple of patches, please apply
‘fix redefinition of child frames on NS’, then ‘Set accessibility
subroles for child frame’ and see if it does what you expect.

This will only work on macOS >= 10.10. I think there must be another
way of doing it on older versions but I haven’t found it yet. I’ll see
if this works before digging any more.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-redefinition-of-child-frames-on-NS.patch --]
[-- Type: text/plain, Size: 1222 bytes --]

From a33d6248a7369fc18111e42b6157a6bf84c5578a Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 13 May 2018 10:33:44 +0100
Subject: [PATCH] Fix redefinition of child frames on NS

* src/nsterm.m (x_set_parent_frame): If the NSWindow has an existing
parent frame, remove it.
---
 src/nsterm.m | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index c8ae31abc0..df883346de 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1958,12 +1958,20 @@ so some key presses (TAB) are swallowed by the system. */
 
   if (p != FRAME_PARENT_FRAME (f))
     {
-      parent = [FRAME_NS_VIEW (p) window];
+      block_input ();
       child = [FRAME_NS_VIEW (f) window];
 
-      block_input ();
-      [parent addChildWindow: child
-                     ordered: NSWindowAbove];
+      if ([child parentWindow] != nil)
+        [[child parentWindow] removeChildWindow:child];
+
+      if (!NILP (new_value))
+        {
+          parent = [FRAME_NS_VIEW (p) window];
+
+          [parent addChildWindow: child
+                         ordered: NSWindowAbove];
+        }
+
       unblock_input ();
 
       fset_parent_frame (f, new_value);
-- 
2.16.1


[-- Attachment #3: 0001-Set-accessibility-subroles-for-child-frame-bug-31324.patch --]
[-- Type: text/plain, Size: 1707 bytes --]

From 8f1502d98ed40ba6c750bd6ab48fb7812d3c937e Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 13 May 2018 11:02:00 +0100
Subject: [PATCH] Set accessibility subroles for child frame (bug#31324)

; Depends on patch in bug#31440.

* src/nsterm.m (x_set_parent_frame): Set subrole depending on whether
frame is a child or not.
---
 src/nsterm.m | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index df883346de..97682f7a47 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1962,7 +1962,15 @@ so some key presses (TAB) are swallowed by the system. */
       child = [FRAME_NS_VIEW (f) window];
 
       if ([child parentWindow] != nil)
-        [[child parentWindow] removeChildWindow:child];
+        {
+          [[child parentWindow] removeChildWindow:child];
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 101000
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 101000
+          if ([child respondsToSelector:@selector(setAccessibilitySubrole:)]
+#endif
+              [child setAccessibilitySubrole:NSAccessibilityStandardWindowSubrole];
+#endif
+        }
 
       if (!NILP (new_value))
         {
@@ -1970,6 +1978,12 @@ so some key presses (TAB) are swallowed by the system. */
 
           [parent addChildWindow: child
                          ordered: NSWindowAbove];
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 101000
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 101000
+          if ([child respondsToSelector:@selector(setAccessibilitySubrole:)]
+#endif
+          [child setAccessibilitySubrole:NSAccessibilityFloatingWindowSubrole];
+#endif
         }
 
       unblock_input ();
-- 
2.16.1

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Fu Yuan]

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

Hi Alan,

Where should I get the Emacs source files?
I was getting those from emacs mirror on GitHub master branch.
Last time I manually edited your fix and noticed that line numbers doesn’t match.
So I guess you are not using HEAD version to create those patches.

What version or commit or branch should I use?
Thanks.

Sincerely, Yuan.

On May 13, 2018, 6:14 PM +0800, Alan Third <alan@idiocy.org>, wrote:
> Please keep the bug tracker Cc’d in.
>
> On Fri, May 11, 2018 at 09:31:14PM +0800, Fu Yuan wrote:
> > I first ran the lisp code and nothing changed. Then I patched source
> > file and compiled, then ran the lisp code on the compiled emacs.
> > Nothing different on that Emacs. The UIElementInspecter shows
> > AXStandardWindow. Of course both time I used “emacs" command with
> > “-Q" flag.
> >
> > Is there anything else I can do?
>
> OK, I looked into this a bit further and it seems this stuff is in the
> accessibility API. I’ve attached a couple of patches, please apply
> ‘fix redefinition of child frames on NS’, then ‘Set accessibility
> subroles for child frame’ and see if it does what you expect.
>
> This will only work on macOS >= 10.10. I think there must be another
> way of doing it on older versions but I haven’t found it yet. I’ll see
> if this works before digging any more.
> --
> Alan Third

[-- Attachment #2: Type: text/html, Size: 2292 bytes --]

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Alan Third]

On Sun, May 13, 2018 at 06:17:10PM +0800, Fu Yuan wrote:
> Hi Alan,
> 
> Where should I get the Emacs source files?
> I was getting those from emacs mirror on GitHub master branch.
> Last time I manually edited your fix and noticed that line numbers doesn’t match.
> So I guess you are not using HEAD version to create those patches.
> 
> What version or commit or branch should I use?

The easiest way is to use git to get the emacs-26 branch, either from
savannah or emacs mirror. Then apply the patches using:

    git am <patch‐file‐name>

This applies the patch as a commit in git.
-- 
Alan Third

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Fu Yuan]

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]

Hi Alan,

I applied the patch and rebuilt Emacs, then ran

    (let ((frame (selected-frame)))
      (make-frame-command)
      (set-frame-parameter nil 'parent-frame frame))

And the subrole of the new frame is now “AXFloatingWindow”.

I think that will do it. It there anything else that I can do?

Sincerely, Yuan.

On May 13, 2018, 6:33 PM +0800, Alan Third <alan@idiocy.org>, wrote:
> On Sun, May 13, 2018 at 06:17:10PM +0800, Fu Yuan wrote:
> > Hi Alan,
> >
> > Where should I get the Emacs source files?
> > I was getting those from emacs mirror on GitHub master branch.
> > Last time I manually edited your fix and noticed that line numbers doesn’t match.
> > So I guess you are not using HEAD version to create those patches.
> >
> > What version or commit or branch should I use?
>
> The easiest way is to use git to get the emacs-26 branch, either from
> savannah or emacs mirror. Then apply the patches using:
>
> git am <patch‐file‐name
>
> This applies the patch as a commit in git.
> --
> Alan Third

[-- Attachment #2: Type: text/html, Size: 2926 bytes --]

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Alan Third]

On Sun, May 20, 2018 at 02:33:28PM +0800, Fu Yuan wrote:
> Hi Alan,
> 
> I applied the patch and rebuilt Emacs, then ran
> 
>     (let ((frame (selected-frame)))
>       (make-frame-command)
>       (set-frame-parameter nil 'parent-frame frame))
> 
> And the subrole of the new frame is now “AXFloatingWindow”.
> 
> I think that will do it. It there anything else that I can do?

Thanks for your help. The only thing that would be helpful for now is
if you could run with this patch applied for a while and look out for
anything unexpected. I don’t think it will cause problems, but you
never know!

Unfortunately, I think this will have to wait until Emacs 26.2.

Eli, should it be added to master then cherry‐picked back to emacs-26
when we’re ready, or am I best just waiting?
-- 
Alan Third

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Eli Zaretskii]

> Date: Tue, 22 May 2018 20:40:02 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: 31324@debbugs.gnu.org
> 
> Unfortunately, I think this will have to wait until Emacs 26.2.
> 
> Eli, should it be added to master then cherry‐picked back to emacs-26
> when we’re ready, or am I best just waiting?

It's up to you, but my recommendation is to commit to master now, and
then cherry-pick after Emacs 26.1 is released (which should be very
soon).

bug#31324: 26.0.91; Wrong AXSubrole of childframe on macOS

[Alan Third]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Tue, 22 May 2018 20:40:02 +0100
>> From: Alan Third <alan@idiocy.org>
>> Cc: 31324@debbugs.gnu.org
>> 
>> Unfortunately, I think this will have to wait until Emacs 26.2.
>> 
>> Eli, should it be added to master then cherry‐picked back to emacs-26
>> when we’re ready, or am I best just waiting?
>
> It's up to you, but my recommendation is to commit to master now, and
> then cherry-pick after Emacs 26.1 is released (which should be very
> soon).

I ended up just leaving this for ages, so I've pushed it to emacs-26 now.
-- 
Alan Third