JMC-5372: Exception printed on page when opening invalid recording

[Joshua Matsuoka]

Hi Alex,

-    public static final String INVALID_RECORDING_DIALOG_TITLE = "Page
could not display data"; //$NON-NLS-1$
-
-    public static final String INVALID_RECORDING_DIALOG_TEXT = "The page
cannot be shown correctly for this recording. The recording may be from an
old JVM, or invalid in some other way."; //$NON-NLS-1$
+    public static final String INVALID_RECORDING_TEXT = "The page cannot
be shown correctly for this recording. The recording may be from an old
JVM, or invalid in some other way."; //$NON-NLS-1$

minor nit/question for Marcus: Should this message be localized in the same
way as all of the other UI messages to keep things consistent? I.E. adding
them to messages.properties and handling them in the usual way.

Cheers,

- Josh

On Tue, Nov 20, 2018 at 4:36 PM Alex Macdonald <almacdon at redhat.com> wrote:

> Ping - I sent this out about a week ago, let me know what you think.
>
> On Wed, Nov 14, 2018 at 4:12 PM Alex Macdonald <almacdon at redhat.com>
> wrote:
>
> > Hi Marcus,
> >
> > On Tue, Nov 13, 2018 at 7:59 PM, Marcus Hirt <marcus.hirt at oracle.com>
> > wrote:
> >
> >> Hi Alex,
> >>
> >> Personally I am a bit allergic to modal dialogs. For example, if we have
> >> multiple recordings ending with problems, I'd rather have the editors
> >> opening
> >> with details than one or more modal dialogs. That is, of course, a
> >> personal
> >> preference that I'd be happy to discuss here. That said, I think what is
> >> shown,
> >> and how it is shown, in the editor could be improved. For example we
> >> should
> >> show the error title and message (that would have been showed in the
> >> modal
> >> dialog) first, followed by the stack trace. Possibly not throwing the
> >> stack trace in the user's face until it is asked for.
> >>
> >
> > That sounds fair enough to me. I've taken what you said into account and
> > made a couple of modifications to the error page.
> >
> > I've transferred the text explaining the error from the dialog to the
> > error page, and removed the dialog modal. The dialog title text was very
> > similar to the current error message printed on the screen, so I've only
> > kept the latter to avoid redundancy. In response to your comment about
> > showing the stack trace when it's asked for, the stack trace is now
> hidden
> > under a expandable component so it can be collapsed & expanded as
> desired.
> >
> > I've included some images to show these changes.
> >
> > Error page (gif): https://imgur.com/g8hpXlg
> > Error page (collapsed): https://imgur.com/irTarA6
> > Error page (expanded): https://imgur.com/M51z8ct
> >
> > Cheers,
> >
> > Alex
> >
> >
> >> Please let me know what you think!
> >>
> >> Kind regards,
> >> Marcus
> >>
> >> On 2018-11-13, 21:45, "jmc-dev on behalf of Alex Macdonald" <
> >> jmc-dev-bounces at openjdk.java.net on behalf of almacdon at redhat.com>
> wrote:
> >>
> >>     Hi,
> >>
> >>     This short patch addresses JMC-5372 [0], in which the error dialog
> is
> >> not
> >>     displayed when an exception is thrown in the JFR Editor.
> >>
> >>     As far as I can tell, the error here is within the catch block of
> the
> >>     "displayPage" function in the JFR Editor [1]. When an exception is
> >> caught
> >>     there is an evaluation of the boolean property for
> >> "showModalErrorDialog"
> >>     [2], however I cannot find the instance where this property would be
> >>     toggled from false to true. As a result, the if-statement takes the
> >> branch
> >>     that displays the error page, but not the error dialog. The proposed
> >> fix
> >>     here removes the check for the boolean (because we have already
> >> caught the
> >>     exception), and instead opt to display both the error page and the
> >> modal.
> >>
> >>     I've included a couple of images to show the result of this patch
> >> [3], as
> >>     well as gifs showing the before [4] & after [5] experience.
> >>     Before (gif): https://imgur.com/hHRmkx3 [3]
> >>     After (gif): https://imgur.com/8rtaysS [4]
> >>     After (img): https://imgur.com/0oBi6nH [5]
> >>
> >>     Thoughts?
> >>
> >>     Cheers,
> >>
> >>     Alex
> >>
> >>     [0] https://bugs.openjdk.java.net/browse/JMC-5372
> >>     [1]
> >>
> >>
> http://hg.openjdk.java.net/jmc/jmc/file/a76a464b3764/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/JfrEditor.java#l240
> >>     [2]
> >>
> >>
> http://hg.openjdk.java.net/jmc/jmc/file/a76a464b3764/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/JfrEditor.java#l243
> >>     [3] https://imgur.com/hHRmkx3
> >>     [4] https://imgur.com/8rtaysS
> >>     [5] https://imgur.com/0oBi6nH
> >>
> >>
> >>
> >>
> >>
> >
>