xpra icon
Bug tracker and wiki

Opened 2 months ago

Last modified 2 weeks ago

#2269 assigned enhancement

HTML5 Improvements

Reported by: Mark Harkin Owned by: Antoine Martin
Priority: major Milestone: 3.0
Component: html5 Version: 2.5.x
Keywords: Cc:

Description

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

Attachments (6)

xpra_menu.png (32.0 KB) - added by Mark Harkin 2 months ago.
Floating Menu
open_windows_menu.png (8.2 KB) - added by Mark Harkin 2 months ago.
open windows menu
html5_pr.zip (1.3 MB) - added by Mark Harkin 2 months ago.
patches.zip (4.9 KB) - added by Mark Harkin 2 months ago.
small patches
float_menu_changes.zip (1.3 MB) - added by Mark Harkin 8 weeks ago.
floating-menu.patch (42.4 KB) - added by Antoine Martin 8 weeks ago.
work in progress patch

Change History (30)

Changed 2 months ago by Mark Harkin

Attachment: xpra_menu.png added

Floating Menu

Changed 2 months ago by Mark Harkin

Attachment: open_windows_menu.png added

open windows menu

comment:1 Changed 2 months ago by Antoine Martin

Owner: changed from Antoine Martin to Mark Harkin

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.

comment:2 Changed 2 months ago by Mark Harkin

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?

comment:3 Changed 2 months ago by Antoine Martin

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.

comment:4 in reply to:  3 Changed 2 months ago by Mark Harkin

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.

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

Changed 2 months ago by Mark Harkin

Attachment: html5_pr.zip added

comment:5 Changed 2 months ago by Mark Harkin

Owner: changed from Mark Harkin to Antoine Martin

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 r22434

comment:6 Changed 2 months ago by Mark Harkin

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

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

comment:7 Changed 2 months ago by Antoine Martin

Owner: changed from Antoine Martin to Mark Harkin

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

  • 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?
  • 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.
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
Last edited 2 months ago by Antoine Martin (previous) (diff)

comment:8 in reply to:  7 Changed 2 months ago by Mark Harkin

Replying to 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.

Changed 2 months ago by Mark Harkin

Attachment: patches.zip added

small patches

comment:9 Changed 2 months ago by Mark Harkin

@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

comment:10 Changed 2 months ago by Antoine Martin

Updates:

  • 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)
  • 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?
  • 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
  • 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.
  • 5 merged in r22461. Do you really need that long? I would have thought that users would have already given up after 10 seconds.
  • 6 merge using a lookup table in r22462.

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

comment:11 in reply to:  10 Changed 2 months ago by Mark Harkin

Replying to Antoine Martin:

Updates:

  • 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

comment:12 Changed 2 months ago by Antoine Martin

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)

Changed 8 weeks ago by Mark Harkin

Attachment: float_menu_changes.zip added

comment:13 Changed 8 weeks ago by Mark Harkin

@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

Changed 8 weeks ago by Antoine Martin

Attachment: floating-menu.patch added

work in progress patch

comment:14 Changed 8 weeks ago by Antoine Martin

Owner: changed from Mark Harkin to Antoine Martin
Status: newassigned

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

Some questions / notes:

  • fullscreen doesn't work for me
  • the handle is a little bit small and hard to grab
  • I can lose the menu if I resize the browser to be too small (menu handle is off-screen)
  • I don't like code like this: this.parentElement.parentElement.parentElement.parentElement.parentElement.parentElement.className="-hide";

Here's an updated work in progress patch with:

  • fixes for svg icons, icon types (you want this server-side fix too: r22547)
  • refactoring of duplicated code (mostly to do with icons)

comment:15 in reply to:  14 Changed 8 weeks ago by Mark Harkin

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

Replying to Antoine Martin:

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

comment:16 Changed 7 weeks ago by Antoine Martin

Updates:

  • merged floating menu in r22549
  • whitespace cleanup in r22550
  • add autohide option for floating menu: r22551
  • don't click through the floating menu when dragging it (partial solution): r22552
  • removed old top bar: r22553

Things left to fix:

  • stop all clicks on the floating menu from propagating to the window that may sit under it (similar to Stop event propagation from jquery Draggable stop to onclick?)
  • the new containment stuff prevents us from moving the floating menu off-screen, but it doesn't keep it within the screen when we shrink the browser window (you have to enlarge the browser window again to find it)

comment:18 Changed 7 weeks ago by Antoine Martin

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).

comment:19 Changed 7 weeks ago by Antoine Martin

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)

comment:20 Changed 7 weeks ago by Mark Harkin

@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

comment:21 Changed 7 weeks ago by Antoine Martin

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

Oops, sorry, try r22567.

comment:22 Changed 7 weeks ago by Mark Harkin

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

comment:23 Changed 7 weeks ago by 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.

comment:24 in reply to:  23 Changed 7 weeks ago by Mark Harkin

Replying to 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.

comment:25 Changed 2 weeks ago by Antoine Martin

See also #2292, #2312, #1844.

Note: See TracTickets for help on using tickets.