lsp-ui icon indicating copy to clipboard operation
lsp-ui copied to clipboard

Fix posframe usage for lsp-ui-doc

Open brotzeit opened this issue 4 years ago • 32 comments

it causes the hover window to capture focus, making editing impossible.

The posframe integration on lsp-ui is slow, due to new frame is being created every time.

brotzeit avatar Jun 26 '20 10:06 brotzeit

In my testing, I didn't run into either of these issues while working on #459. If anyone is experiencing these issues, I'd like to get some more details on the environment this is occuring in so I can try and reproduce these.

Sorixelle avatar Jun 27 '20 02:06 Sorixelle

This happens to me on macOS 10.15.5: GNU Emacs 26.3 (build 1, x86_64-apple-darwin18.2.0, NS appkit-1671.20 Version 10.14.3 (Build 18D109)) of 2019-09-02. I don't have any customizations for lsp-ui or posframe group.

ezgif com-video-to-gif

Here I move pointer to a method name, lsp-ui-doc pops up and it has focus. I press q to switch focus back to the original frame.

dmakarov avatar Jun 27 '20 06:06 dmakarov

@alanz are you also using macos?

yyoncho avatar Jun 27 '20 06:06 yyoncho

Did some testing, and managed to get to the bottom of the focus issues. It seems to be an issue with Cocoa builds of Emacs. This issue doesn't exist in the emacs-mac port, so the solution would be to use this. It seems like this is an Emacs bug, and completely out of our control. More info in this issue in posframe.

Sorixelle avatar Jun 27 '20 06:06 Sorixelle

It seems like this is an Emacs bug, and completely out of our control

It was working in the previous implementation, right?

yyoncho avatar Jun 27 '20 06:06 yyoncho

Just reverted the commit, it seems like it did work on Cocoa builds. I'm still unconvinced this is a problem on our side however, as I don't see any reason our code would cause the new frame to get focus.

Sorixelle avatar Jun 27 '20 07:06 Sorixelle

This may be a macOS cocoa issue, but the previous non-posframe implementation did work.

dmakarov avatar Jun 27 '20 07:06 dmakarov

I've come up with a solution that works for Emacs 26 on macOS. However, this solution prevents using the mouse to focus the frame in Linux, and doesn't fix the issue in Emacs 27 or 28 on macOS (no clue why). Also of note is that in Emacs 28 in macOS, it seems to completely ignore positioning and just display in the top-left corner of the screen. If we're ok with losing the ability to focus with the mouse outside of macOS, I'll make a PR with this fix.

The solution is to not advise posframe--redirect-frame-focus, and just let it setup the focus redirect. lsp-ui-doc-focus-frame still works, but the mouse can still focus the frame in Emacs 26 for some reason.

tl;dr - Emacs on macOS is really, really weird.

EDIT: The Emacs 27/28 issues might have something to do with the obsoletion of focus-in-hook for after-focus-change-function. That should probably be reported to posframe.

Sorixelle avatar Jun 27 '20 07:06 Sorixelle

IMO not getting the focus is better than getting the focus all the time. Ideally, we should not introduce regression compared to the previous behaviour of the popup.

yyoncho avatar Jun 27 '20 08:06 yyoncho

@alanz are you also using macos?

Yes, the problem shows up on my work machine, macos. But not on my home machine, Debian Testing.

alanz avatar Jun 27 '20 09:06 alanz

@Sorixelle About this

The posframe integration on lsp-ui is slow, due to new frame is being created every time.

A simple profiling will reveal that

- #<compiled 0x347b065>                                                                         266  88%
 - lsp--parser-on-message                                                                       266  88%
  - #<compiled 0x4297c65>                                                                       266  88%
   - #<compiled 0x4297c49>                                                                      266  88%
    - #<compiled 0x4297c15>                                                                     266  88%
     - apply                                                                                    266  88%
      - #<compiled 0x4294071>                                                                   266  88%
       - lsp-ui-doc--callback                                                                   266  88%
        - lsp-ui-doc--display                                                                   266  88%
         - lsp-ui-doc--make-frame                                                               265  88%
          - posframe-show                                                                       260  86%
           - posframe--create-posframe                                                          255  85%
            - make-frame                                                                        255  85%
             - frame-creation-function                                                          255  85%
              - apply                                                                           255  85%
               - #<compiled 0x1072a75>                                                          255  85%
                + x-create-frame-with-faces                                                     255  85%
             posframe--set-frame-position                                                         2   0%
             posframe--mouse-banish                                                               1   0%
          + lsp-ui-doc--delete-frame                                                              5   1%
+ redisplay_internal (C function)                                                                25   8%
+ timer-event-handler                                                                             5   1%
+ ...                                                                                             2   0%
+ eldoc-pre-command-refresh-echo-area                                                             1   0%

It calls make-frame on every lsp-ui-doc--display request, which is slow on some system. Even the default implementation of posframe try to avoid that by associating an posframe--frame with every posframe buffer so it will not be created again.

In case of lsp-ui, why this happens is because of this check inside lsp-ui-doc--display.

      (when (or (not lsp-ui-doc-use-webkit)
                (not (lsp-ui-doc--get-frame)))
        (lsp-ui-doc--set-frame (lsp-ui-doc--make-frame)))

So just simply not set lsp-ui-doc-use-webkit will make this check passed and calling lsp-ui-doc--make-frame. Inside lsp-ui-doc--make-frame, it intentionally delete the posframe with lsp-ui-doc--delete-frame and causes new frame to be created everytime. I believe this check is mistyping and should change from or to and, or simply change to use unless and remove not instead of when.

However, just changing that will not work since the posframe position will not be updated. In the original code, the frame position is updated in every call to lsp-ui-doc--display with lsp-ui-doc--move-frame, we need to update that with posframe too (call posframe-show with new position, I guess).

Alternatively, change the check to check for lsp-ui-doc-use-webkit only and remove the delete frame from lsp-ui-doc--make-frame may work too. We will still calling stuff of setting up frame parameters every time though.

kiennq avatar Jun 28 '20 01:06 kiennq

I have this problem on NixOS, so the issue is definitely not OSX-specific. But I'm using xmonad, so it might be a contributing factor.

omidmnz avatar Jun 28 '20 14:06 omidmnz

Same issue here after I upgraded today. It's pleasure to provide details if need.

seagle0128 avatar Jun 28 '20 15:06 seagle0128

IMO, if this issue is causing significant problems to some users, we should revert the PR. We can always re-land it later when it's more stable.

danielmartin avatar Jun 28 '20 16:06 danielmartin

@danielmartin I think that's the correct thing to do now, since package.el does not officially support downgrades.

omidmnz avatar Jun 28 '20 16:06 omidmnz

Yeah, I'd be in favour of reverting it for now. Didn't expect it to be such a buggy mess when it was working with no issues at all on my system 😅

Sorixelle avatar Jun 29 '20 01:06 Sorixelle

I also have this problem on linux with xmonad :)

kurnevsky avatar Jun 29 '20 13:06 kurnevsky

Thanks @kurnevsky for answering. Can anyone who has problem with mouse being focused check the value of focus-follow-mouse? I think that's the problem causing mouse is being automatically focus into posframe

kiennq avatar Jun 30 '20 01:06 kiennq

focus-follows-mouse is a variable defined in ‘C source code’.
Its value is nil

I'm not sure if this helps, but I am using flycheck-posframe without this problem.

P.S.: My xmonad config has focus-follows-mouse disabled too.

omidmnz avatar Jun 30 '20 01:06 omidmnz

Another bug I noticed - when I switch buffer when posframe is about to be shown - it's shown in another window with an error

Error processing message (wrong-type-argument frame-live-p #<dead frame  0x6649010>).

kurnevsky avatar Jun 30 '20 09:06 kurnevsky

I've come up with a solution that works for Emacs 26 on macOS. However, this solution prevents using the mouse to focus the frame in Linux, and doesn't fix the issue in Emacs 27 or 28 on macOS (no clue why). Also of note is that in Emacs 28 in macOS, it seems to completely ignore positioning and just display in the top-left corner of the screen. If we're ok with losing the ability to focus with the mouse outside of macOS, I'll make a PR with this fix.

The solution is to not advise posframe--redirect-frame-focus, and just let it setup the focus redirect. lsp-ui-doc-focus-frame still works, but the mouse can still focus the frame in Emacs 26 for some reason.

tl;dr - Emacs on macOS is really, really weird.

EDIT: The Emacs 27/28 issues might have something to do with the obsoletion of focus-in-hook for after-focus-change-function. That should probably be reported to posframe.

is there any hack to do in emacs 27/28 while something is fixed in posframe ?

d1egoaz avatar Jul 09 '20 18:07 d1egoaz

BTW, this works for me: https://github.com/emacs-lsp/lsp-ui/issues/414#issuecomment-601216646

but it adds some screen screen flickering at the top

d1egoaz avatar Jul 09 '20 18:07 d1egoaz

I've come up with a solution that works for Emacs 26 on macOS. However, this solution prevents using the mouse to focus the frame in Linux, and doesn't fix the issue in Emacs 27 or 28 on macOS (no clue why). Also of note is that in Emacs 28 in macOS, it seems to completely ignore positioning and just display in the top-left corner of the screen. If we're ok with losing the ability to focus with the mouse outside of macOS, I'll make a PR with this fix.

The solution is to not advise posframe--redirect-frame-focus, and just let it setup the focus redirect. lsp-ui-doc-focus-frame still works, but the mouse can still focus the frame in Emacs 26 for some reason.

tl;dr - Emacs on macOS is really, really weird.

EDIT: The Emacs 27/28 issues might have something to do with the obsoletion of focus-in-hook for after-focus-change-function. That should probably be reported to posframe.

@Sorixelle - can you post your findings?

yyoncho avatar Aug 01 '20 17:08 yyoncho

@Sorixelle - can you post your findings?

cc @ericdallo who will take a look.

yyoncho avatar Aug 01 '20 17:08 yyoncho

Another xmonad user on Linux here. I ran into the same problem with the lsp-ui-doc-frame (sp?) stealing focus. It only seemed to happen if my mouse cursor was in a position that was overlaying the frame.

I tried both of the following in my setup, and then restarting Emacs. Either one worked to solve this problem (the doc frame doesn't get the focus now).

(setq focus-follows-mouse 'auto-raise)

or:

(setq focus-follows-mouse t)

Version info: GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.21, cairo version 1.17.3) of 2020-07-26

joncol avatar Aug 01 '20 19:08 joncol

Same problem here on Fedora 32 with both XMonad and Gnome 3, so it doesn't seem to be XMonad specific. @joncol's fix didn't work for me.

Additionally, the focus does not get stolen on a fresh Emacs. I need to have set the focus to the doc popup once first, say, by clicking my mouse, or scrolling. This is also the case when opening new frames: when I first open a new frame (even if it's the same buffer) the doc popup does not steal my focus.

But once I've clicked into the doc popup even once, I cannot undo it, it seems, and every successive doc popup will steal the focus if it pops up under my mouse pointer. I cannot call lsp-ui-doc-hide to unset the focus stealing behaviour.

If I were to guess, I'd say there's some persistent state somewhere that gets set as soon as the doc frame is focused for the first time.

So I guess that's another workaround: open a new frame, close the current one.

Ah, but I did find another workaround: if I manually call lsp-ui-doc-unfocus-frame the focus stealing behaviour also ceases. Until I (accidentally) click into the doc popup again!

adimit avatar Aug 02 '20 09:08 adimit

So I guess that's another workaround: open a new frame, close the current one.

This is not an option because creating a new frame is a slow operation. But we could call lsp-ui-doc-unfocus-frame before showing the popup.

yyoncho avatar Aug 02 '20 10:08 yyoncho

Yeah, I meant it as a user-level workaround. Methinks calling lsp-ui-doc-unfocus-frame is the right way to go. I don't quite understand what select-frame-set-input-focus does that just clicking in the right frame doesn't, but that seems to be doing the trick (for me.)

adimit avatar Aug 02 '20 11:08 adimit

An added bit of data: it seems to happen to me with all posframes, not just LSP ui doc. This includes company-posframe. I think it's just more noticable with the latter, as it's bigger in dimension.

So it's possibly a posframe issue, or a posframe/xmonad issue.

adimit avatar Sep 13 '20 19:09 adimit

@joncol's solution works (tack!) until I give the frame focus (Arch/Xmonad) but I found that running lower-frame solves the issue until I click the frame again :slightly_smiling_face:

andrelaszlo avatar Oct 22 '20 17:10 andrelaszlo