Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 316950 - Ctrl+V goes to a wrong handler on the "Add Repository"dialog
Summary: Ctrl+V goes to a wrong handler on the "Add Repository"dialog
Status: RESOLVED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 1.0   Edit
Hardware: All All
: P3 critical (vote)
Target Milestone: 1.0 RC2   Edit
Assignee: Remy Suen CLA
QA Contact: Paul Webster CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-15 13:30 EDT by Oleg Besedin CLA
Modified: 2010-07-06 16:25 EDT (History)
4 users (show)

See Also:
remy.suen: review+


Attachments
ShellActivationListener patch v1 (1.57 KB, patch)
2010-07-06 15:02 EDT, Remy Suen CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oleg Besedin CLA 2010-06-15 13:30:35 EDT
Using build I20100614-2011 Ctrl+V when pressed Ctrl+V to paste new P2 location on the "Add Repository" dialog, the paste goes to the last active Eclipse part.

To duplicate:

- Help -> Install new software; select "Add"
- Put cursor in the "Location" field, make sure there is some text in the clipboard (for instance, "download.eclipse.org")
- Press Ctrl+V - notice that paste goes to the last active Eclipse part, in my case, Package Explorer
Comment 1 Oleg Besedin CLA 2010-06-21 11:43:06 EDT
Its pretty bad as 1) a person tas to type in P2 site URIs - can't copy/paste; 2) this causes a bunch of NPEs later on, including immortal modal dialog.
Comment 2 Remy Suen CLA 2010-06-21 21:13:48 EDT
When the 'Add...' button is clicked, the active part is flagged as one can see from the blue gradient background.

When the nested modal dialog is about to appear, the outer modal dialog is deactivated, altering the active context chain to once again point down to the part. When the modal dialog itself is activated, we ask the parent context (the outer modal dialog) to activate the nested dialog's context. This is done but this change is not propagated upwards. So you get something like...

Application (active)
+MWindow (active)
++MPerspective (active)
+++MPart (active)
++Outer modal shell
+++Nested modal shell (active)

...which causes the keybindings to be delivered to the part instead.
Comment 3 Remy Suen CLA 2010-06-22 06:36:48 EDT
(In reply to comment #2)
> So you get something like...
> 
> Application (active)
> +MWindow (active)
> ++MPerspective (active)
> +++MPart (active)
> ++Outer modal shell
> +++Nested modal shell (active)
> 
> ...which causes the keybindings to be delivered to the part instead.

To be clear, the expected sequence would be...

Application (active)
+MWindow (active)
++MPerspective
+++MPart (active)
++Outer modal shell (active)
+++Nested modal shell (active)

...of course, this presents the problem we see right now wherein the tab folder stays white even though we'd expect there to still be an "active part".
Comment 4 John Arthorne CLA 2010-07-02 11:23:36 EDT
This still occurs in I20100701-1105. It is not only Paste, but also Home, End, etc do not work in this dialog.
Comment 5 Remy Suen CLA 2010-07-06 15:02:20 EDT
Created attachment 173587 [details]
ShellActivationListener patch v1

This patch implements the propagation fix described by comment 2 and also ensures that we are never pointing at a disposed context for an active child.

Paul, what do you think?
Comment 6 Paul Webster CLA 2010-07-06 15:44:20 EDT
(In reply to comment #5)
> Created an attachment (id=173587) [details]
> ShellActivationListener patch v1
> 
> Paul, what do you think?

This makes perfect sense, and I gave it a whirl.  It also seems self correcting.

Could you make a quick test of some of our main dialogs like Commit, Search, and breakpoint properties/conditional breakpoints for things like content assist, Home, End, CTRL+ARROW.

If it seems  fine, commit it and I'll do a build submission just before the build tonight at around 19:00

PW
Comment 7 Remy Suen CLA 2010-07-06 16:24:33 EDT
(In reply to comment #6)
> Could you make a quick test of some of our main dialogs like Commit, Search,
> and breakpoint properties/conditional breakpoints for things like content
> assist, Home, End, CTRL+ARROW.

Content assist in the breakpoint and search dialogs and home/end stuff seems to be fine.

Fix released to HEAD.