Xpra-org / xpra-html5

HTML5 client for Xpra
Mozilla Public License 2.0
211 stars 55 forks source link

HTML5 Improvements #69

Closed totaam closed 3 years ago

totaam commented 5 years ago

Issue migrated from trac ticket # 2269

component: html5 | priority: major | resolution: worksforme

2019-04-10 11:42:49: mjharkin created the issue


I've been working on improving the HTML5 client for our own needs, namely:

  • minimally invasive draggable menu (to replace top bar)
  • window selection menu
  • start menu
  • small ui changes (dark background, white window headers)

It's functional but rough around the edges. Not sure if you're interested in taking it on? I can tidy it up a bit more if you are.

There's a lot of file changes here so I'd recommend copying from head at: [https://github.com/mjharkin/Xpra/tree/dev]

totaam commented 5 years ago

2019-04-10 11:43:29: mjharkin uploaded file xpra_menu.png (32.0 KiB)

Floating Menu xpra_menu.png

totaam commented 5 years ago

2019-04-10 11:43:40: mjharkin uploaded file open_windows_menu.png (8.2 KiB)

open windows menu open_windows_menu.png

totaam commented 5 years ago

I would totally take all of the above if you can submit it. It might be a lot easier to merge before the refactoring and move to typescript.

totaam commented 5 years ago

2019-04-10 12:45:41: mjharkin commented


Great, I'll tidy up and make a few more changes and hopefully submit a patch sometime next week.

Do you want to keep the top bar as an option alongside these changes or can I replace it?

totaam commented 5 years ago

Great, I'll tidy up and make a few more changes and hopefully submit a patch sometime next week.

Excellent, thanks!

Do you want to keep the top bar as an option alongside these changes or can I replace it?

I'm not sure yet. I was thinking of adding a few more items in the top bar, but maybe this won't be needed. Sorry, I don't know at this point.

totaam commented 5 years ago

I was thinking of adding a few more items in the top bar, but maybe this won't be needed.

Sorry, I don't know at this point.

No problem, I'll keep them both as mutex options.

totaam commented 5 years ago

2019-04-16 15:00:51: mjharkin uploaded file html5_pr.zip (1345.0 KiB)

totaam commented 5 years ago

2019-04-16 15:10:31: mjharkin commented


There quite a lot here, I'm hoping you'll accept it all. The git diff is at https://github.com/mjharkin/Xpra/commit/7d5a4f0e3a84cc007f831480bfcc7934401625b7 I'll try and list the changes:

  • connect.html hitting enter on password field submits form
  • double click on window title bar toggles maximize
  • local minimize windows (not sending to server yet)
  • fix for mouse events happening on windows under other windows title bar
  • screen background now dark blue with white windows
  • fix for background in fullscreen
  • open url directly and fallback to notification if blocked by popup-blocker
  • es6-shim updated for IE11 compat (more log cats) in connect.html
  • Danish browser language mapping ("da" mapped to "dk")
  • workaround for Java JIDE popups forced to "NORMAL" window

float menu bar:

  • draggable and autohides
  • main menu with basic functions (taken from top_bar) and start menu
  • open window menu
    • holding ctrl over menu allows you to focus on hover
    • otherwise click to focus
    • minimize, maximize, close from menu
  • support tray icons

I think the easiest way to submit this was zipping the html5 folder (html5_pr.zip​). the code was branched from 22434

totaam commented 5 years ago

2019-04-16 20:54:07: mjharkin commented


Sorry, already one quick patch needed on top of the zip, forgot to test taskbar mouse actions.

https://github.com/mjharkin/Xpra/commit/0dcea2f807c86abe03c7bb55a9627e1d833acf4e

totaam commented 5 years ago

I've started merging some of this stuff (ignoring quite a bit of whitespace or line swaps helped):

I haven't merged the submit-on-enter-in-password field because, that already works in 2.5

As for the rest, something isn't working. Can you rebase and submit more piecemeal patches? Splitting up the different changes, ie:

totaam commented 5 years ago

2019-04-17 12:49:38: mjharkin commented


Replying to [comment:7 Antoine Martin]:

I've started merging some of this stuff (ignoring quite a bit of whitespace or line swaps helped): That's weird, I thought I cleaned up any whitespace at least in the git diff, submitted from a Linux box so maybe it's a line endings issue.

  • r22438 es6-shim update for IE11
  • r22439 maximize on header double click
  • r22440 convert favicon from string to Uint8Array if needed - I assume this is needed for IE or something? Yeah think this was an IE issue
  • r22442 open URL: try to use a popup, only show the notification when that fails

I haven't merged the submit-on-enter-in-password field because, that already works in 2.5

As for the rest, something isn't working. Strange, definitely working on head at​https://github.com/mjharkin/Xpra/tree/dev Can you rebase and submit more piecemeal patches? Splitting up the different changes, ie:

  • css changes
  • floating menu (minimal bits)
  • minimize, etc
  • start-command xdg menu (and constify 30 somewhere)
  • JidePopup workaround

Yeah I'll split it up as best i can and resubmit, probably towards the end of next week due to other commitments.

totaam commented 5 years ago

2019-04-18 07:40:08: mjharkin uploaded file patches.zip (4.9 KiB)

small patches

totaam commented 5 years ago

2019-04-18 07:45:58: mjharkin commented


@Antoine Martin Can you try these patches (attached) for the small stuff and then I'll rebase again for the floating menu changes. Patches are from git so hopefully they should work. 1 New window theme 2 Jide popup fixes 3 Fix for no mouse events on windows that aren't in focus 4 Force current window focus on server 5 Increase hello timeout as this happens alot with proxy server 6 Danish language fixes (double entry in keycodes not sure how to properly handle this) 7 Catch incorrect window resize event

totaam commented 5 years ago

Updates:

PS: your patches were in UTF-16LE, not the easiest format to deal with. I've converted them using:

iconv -f UTF-16LE -t UTF-8 2.patch > 2-utf8.patch
totaam commented 5 years ago
  • 1: r22459 new window theme + move background image to css, I also moved the border definition to css rather than adding the style in-line in javascript (new class=border)

Great, thanks.

  • 2: not applied the "Jide popup fixes" yet, can you give me a sample application to reproduce? this looks like a symptom of a modal bug of some sort - maybe we can fix that more reliably without hardcoding a specific window title?

I'll try and get something togther next week

  • 3: applied the window_set_focus part only: the other part of the patch would prevent pointer tracking on a window which doesn't have focus, it would be better to track the window-move event and toggle a flag during that time

Sorry didn't realize mouse events are allowed on unfocused windows. I'll try and get something that's specific to window titlebars for next week

  • 4: this was added in r8730, there must have been a reason for the dont call set focus unless the focus has actually changed - how do I reproduce the problem? Setting the focus unnecessarily is not much of a problem since clicks are sparse events, but I would rather fix the root cause if we can find it.

It's a java windows that is shown, removed and shown again very quickly. I'll try and get a test case together for it.

  • 5 merged in r22461. Do you really need that long? I would have thought that users would have already given up after 10 seconds.

Probably not but my proxy spawned servers take quite a while, 30secs would probably be ok.

  • 6 merge using a lookup table in r22462. Thanks. Any idea on the other part of it? I just want Oslash but Ooblique is being sent. I just reordered to send Oslash instead.

PS: your patches were in UTF-16LE, not the easiest format to deal with. I've converted them using:

iconv -f UTF-16LE -t UTF-8 2.patch > 2-utf8.patch

I'll try and diff as UTF-8 or convert before uploading next time

totaam commented 5 years ago

Sorry didn't realize mouse events are allowed on unfocused windows.

Try r22463. It should be better already.

.. 30secs would probably be ok.

Reduced to 30s in r22464.

Any idea on the other part of it?

Oh right, forgot about that one. Done: r22465. (I wished there was a better way... keyboard mapping is always hit or miss, mostly miss)

totaam commented 5 years ago

2019-04-25 09:33:06: mjharkin uploaded file float_menu_changes.zip (1345.0 KiB)

totaam commented 5 years ago

2019-04-25 09:42:42: mjharkin commented


@Antoine Martin Can you try and overlay float_menu_changes.zip on the html5 folder. (Don't think I can create a patch with binary files)

This is rebased from r22546 and should contain "the rest", float menu, start menu and minimize. excluding the patches previously submitted. Github diff is here: https://github.com/mjharkin/Xpra/commit/5b8faf48dbe13c0967ceec205ebf7558e792b461

totaam commented 5 years ago

2019-04-25 16:38:51: antoine uploaded file floating-menu.patch (42.4 KiB)

work in progress patch

totaam commented 5 years ago

I like this a lot, it works beautifully so I think I will drop the top bar.

Some questions / notes:

Here's an updated work in progress patch with:

totaam commented 5 years ago

2019-04-25 19:09:53: mjharkin commented


These changes are relatively small so just to be quick I'll point you to the github diffs:

I like this a lot, it works beautifully so I think I will drop the top bar.

Great, yeah I don't think the top bar is needed after this.

  • fullscreen doesn't work for me

Damn, was a little eager with the clean up, fix is here: https://github.com/mjharkin/Xpra/commit/6c5517db362b333af5ffe91ad5821dedd1909cf5

  • the handle is a little bit small and hard to grab

Increased to 20px and a little rework here: https://github.com/mjharkin/Xpra/commit/fe6b046ae912081a7e4d3e1d109aa2c2842475fc

  • I can lose the menu if I resize the browser to be too small (menu handle is off-screen)

Constrained here: https://github.com/mjharkin/Xpra/commit/ccc8037abc1a0226e210ab6ffdb40fb53394a561

  • I don't like code like this: this.parentElement.parentElement.parentElement.parentElement.parentElement.parentElement.className="-hide";

Agreed, a super grandparent!, changed here: https://github.com/mjharkin/Xpra/commit/ae69e1f5bff7069baf3b615974e6012472d0f3c0

totaam commented 5 years ago

Updates:

Things left to fix:

totaam commented 5 years ago

With r22556, the tray shows up again, but clicking on it doesn't do anything. And when "autohide" is enabled, the tray icon cannot be clicked on as the line wraps outside the floating menu.

r22557 fixes fullscreeen (was using the top-bar element, now removed).

totaam commented 5 years ago

The tray location was not getting synced, fixed in r22558. They needed a little bit more space to prevent wrapping: r22559. (I profoundly dislike adding hard-coded values like this.. but couldn't find a better fix)

totaam commented 5 years ago

2019-04-30 10:47:08: mjharkin commented


@Antoine

I tried to do some testing on this but it looks like the tray is broken server side in the latest centos beta and possibly also head.

Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/x11/gtk_x11/tray.py", line 202, in do_xpra_client_message_event
    window = x11_foreign_new(xid)
  File "/usr/lib64/python2.7/site-packages/xpra/gtk_common/gtk_util.py", line 484, in x11_foreign_new
    return gdk.window_foreign_new_for_display(xid)
TypeError: window_foreign_new_for_display() argument 1 must be gtk.gdk.Display, not int
totaam commented 5 years ago

TypeError: window_foreign_new_for_display() argument 1 must be gtk.gdk.Display, not int

Oops, sorry, try r22567.

totaam commented 5 years ago

2019-05-02 08:21:46: mjharkin commented


Tray and autohide are still a bit messy I'll come back to this, but here's a potential fix for mouse events on underlying windows: https://github.com/mjharkin/Xpra/commit/400348d232cc519bb08101b7f6fdb6b0198a083e

totaam commented 5 years ago

here's a potential fix for mouse events on underlying windows:

Hmm, wouldn't that prevent ALL motion events from being reported against the root? Some applications (ie: xeyes) want to track the pointer even when it isn't confined to their own window.

totaam commented 5 years ago

2019-05-02 12:06:52: mjharkin commented


Replying to [comment:23 Antoine Martin]:

here's a potential fix for mouse events on underlying windows: Hmm, wouldn't that prevent ALL motion events from being reported against the root? Some applications (ie: xeyes) want to track the pointer even when it isn't confined to their own window. Yeah fair enough, I'm sure there's a better way of doing this and I'm just hacking now but something like this would be close but would need somthing similar for titlebar buttons.

  var targetId = e.target.id;
  if(targetId.startsWith("head")){
      wid = targetId.replace("head", '');
  }else if(targetId.startsWith("title")){
      wid = targetId.replace("title", '');
  }else if(targetId.startsWith("windowicon")){
      wid = targetId.replace("windowicon", '');
  }else if(targetId!="screen" && e.target.localName != "canvas"){
      return;
  }

I'm trying to stop the mouse events hitting the window under another windows titlebar, so I guess the events should be send to the window of that titlebar instead... and then of course avoid window events when using the float menu.

totaam commented 5 years ago

2019-06-03 12:06:16: antoine commented


See also Xpra-org/xpra#2292, Xpra-org/xpra#2312, Xpra-org/xpra#1844.

totaam commented 5 years ago

The help I was hoping to get did not materialize, so I don't have time to take this further... sorry. This will have to do for 3.0, will follow up in Xpra-org/xpra-html5#15

totaam commented 5 years ago

2019-09-13 19:04:45: sali.andrew commented


We also ran into the titlebar "click through" issue. A workaround we are using is to add the following in Window.js at the end when the window is created (right after the "// assign some interesting callbacks" comment):

jQuery(this.div).mousedown(function (e) {
          e.stopPropagation();
      })

This solves the issue as the "ghost click" is created from the titlebar click propagating up the parent to "#screen" where an event listener sends it to the server. Using the above the event is not propagated from the window to the screen div and there are no "phantom clicks". Clicks landing outside the div are not affected. Clicks inside the div are not affected.

totaam commented 5 years ago

This solves the issue as the "ghost click" is created from the titlebar click propagating up the parent to "#screen" where an event listener sends it to the server.

Awesome, thank you! Applied in r23788 with one minor change: this change stops both mousedown and mouseup from propagating, the vast majority of applications won't handle mouseup if they don't first see a mousedown, but some do.

totaam commented 5 years ago

Applied in r23788 with one minor change: this change stops both mousedown and mouseup from propagating, the vast majority of applications won't handle mouseup if they don't first see a mousedown, but some do.

The minor change caused a bug: Xpra-org/xpra#2418, so that part got reverted in r23826.