SV: JMC-6313: Exit menu item alternates visibility in "File"dropdown menu

[Marcus Hirt]

Looks good!

Kind regards,
Marcus

-----Ursprungligt meddelande-----
Från: jmc-dev <jmc-dev-bounces at openjdk.java.net> För Alex Macdonald
Skickat: den 10 januari 2019 20:11
Till: Marcus Hirt <marcus.hirt at oracle.com>
Kopia: jmc-dev at openjdk.java.net
Ämne: Re: JMC-6313: Exit menu item alternates visibility in "File"dropdown menu

Hi Marcus,

On Thu, Jan 10, 2019 at 1:25 PM Marcus Hirt <marcus.hirt at oracle.com> wrote:

> Hi Alex,
>
> Looks good. Perhaps inline the recentText and recentId String local 
> variables in ApplicationActionBarAdvisor? Let me know if you need help 
> running the UI tests.
>

Sure that sounds good to me, I've updated it in a newer webrev @ http://cr.openjdk.java.net/~aptmac/JMC-6313/webrev.01/.

I've run the UI tests with this patch applied and it doesn't interfere with the tests or menu highlighting.

Kind regards,
> Marcus
>
>
> On 2019-01-10, 18:52, "jmc-dev on behalf of Alex Macdonald" < 
> jmc-dev-bounces at openjdk.java.net on behalf of almacdon at redhat.com> wrote:
>
>     Hi,
>
>     The following webrev [0] addresses issue JMC-6313 [1], in which 
> the Exit
>     option in the menubar toggles visibility.
>
>     The culprit here is the recent files list, which is supplied by the
>     ReopenEditorMenu via Eclipse Platform UI [2]. When added to a menu, the
>     ReopenEditorMenu has an interesting effect where subsequent menu items
>     alternate being included and being missing from the menu when the 
> menu is
>     opened. I cloned the Platform UI repo to try and poke around, and I ran
>     into an issue where the subsequent menu items actually aren't 
> added to the
>     menu every-other click. I recreated this behaviour on a simple RCP
>     application and opened a bug report on the Eclipse bugzilla [3].
>
>     For what it's worth, the Recent Files list should be added to it's own
>     submenu. Placing the list into it's own submenu is also a workaround
>     because we won't be adding an Exit action sequentially in the menu 
> after
>     the Recent Files list. The included patch [0] adds a new submenu
>     "recentMenu" and adds a couple of separators to distinguish it 
> from it's
>     neighbouring menu items.
>
>     Images [4]:
>     I've included a couple of images of before [5] and after [6] this 
> patch, as
>     well as a couple of gifs (before [7] & after [8]) that show the 
> issue and
>     proposed solution.
>
>     Thoughts?
>
>     Cheers,
>
>     Alex
>
>     [0] http://cr.openjdk.java.net/~aptmac/JMC-6313/webrev.00/
>     [1] https://bugs.openjdk.java.net/browse/JMC-6313
>     [2]
>
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.workbench/Eclipse%20UI/org/eclipse/ui/internal/ReopenEditorMenu.java
>     [3] https://bugs.eclipse.org/bugs/show_bug.cgi?id=543269
>     [4] https://imgur.com/a/pJkEoKQ
>     [5] https://imgur.com/gnV5aFw
>     [6] https://imgur.com/8S4L7wi
>     [7] https://imgur.com/GlVZ0Ut
>     [8] https://imgur.com/jh6lDoH
>
>