On Fri, 4 Apr 2025 21:28:59 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> hmm, not sure if I share the concern (did I understand the concern?)
>>
>> the idea is to have a low-level method `takeScreenshotBase64(prefix,
>> postfix)` that can be used in non-standard situations, but also provide the
>> convenience method that will be used in the tests, `writeScreenshotBase64()`.
>
>> hmm, not sure if I share the concern (did I understand the concern?)
>>
>> the idea is to have a low-level method `takeScreenshotBase64(prefix,
>> postfix)` that can be used in non-standard situations, but also provide the
>> convenience method that will be used in the tests, `writeScreenshotBase64()`.
>
> That part sounds fine. However, even in non-standard situations, the caller
> of `takeScreenshotBase64` should not be expected to pass
> `"data:image/png;base64,"` to that method. It is the method itself that knows
> that the format is PNG, so that method should provide that part of the string
> when converting it to a base64 screenshot. So, for example:
>
>
> takeScreenshotBase64(null, null); <-- returns a data URL of a
> base64-encoded PNG
> takeScreenshotBase64("{ screenshot:", "}"); <-- return the same with the
> prefix and postfix
>
>
> So what I'm trying to say is that `"data:image/png;base64,"` is an integral
> part of the base64-encoded image, not an optional prefix that a caller must
> pass in.
One thing that happened recently (in the past 10 years or so) is that JSON
became a frequently used format for logs. It's not perfect, but it's easy to
parse. With that, json-oriented log viewers came.
We don't have a consistent way to log data (or show data in `.toString()`). A
good log viewer can pretty print an object in the log file, decode a hex- or
base-64 encoded string, or even run a query on a log file.
We are not ready for JSON logs, I admit, but this was the rationale behind the
design of this class:
- a low-level method that returns byte[]
- a base-64 encoding method that allows for custom prefix/suffix to be able to
do a data url or a json
- a convenience method to use in tests
Also, considering what you said about gradle and stderr, perhaps
`writeScreenshot()` should always emit to stderr.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1746#discussion_r2029466429
