Changing styling of how ImageDocument displays images to match MediaDocument?

[dfalc...@chromium.org]

Hey all,

As part of adding a new download manager UI to Chrome on Android, I've been investigating how standalone images & video are shown in tabs (e.g. for viewing files stored locally on your device or selecting "open image in new tab" from the context menu).  I tracked down the page generation to ImageDocument.cpp and MediaDocument.cpp, but was wondering about the inconsistency between the two: images are drawn starting in the top-left with a white background, while media is shown centered on the screen with a black background.

Are there any historical reasons why this works this way?  Would there be strong pushback if I suggested changing images to be presented in the same way as videos (centered on black, maybe zoomed to fit everything on screen when it's first opened)?

Thanks!

[Elliott Sprehn]

Seems good to me! Note that the shape of the document body is actually required per spec, but we can use a UA shadow root here.

Btw I think you're cross posting between public and private mailing lists. :)

[Philip Jägenstedt]

AFAIK there's no good reason for the difference, it's just an historical accident. ImageDocument is a pretty terrible image viewer, it could be made a lot better for large images in particular.

Even if UA shadow roots are used, I think that'd be observable in that any attempt by the page to attach another shadow root would then fail. Pretty obscure, but I think it'd be worth investigating if these documents actually need to be considered same-origin with the page that embeds them.

One might also consider using different documents for iframes and top-level frames.

[Philip Rogers]

+1

I added the ImageDocument UseCounter a few months ago for this purpose. The count is pretty high, 0.6% of pageviews: https://www.chromestatus.com/metrics/feature/timeline/popularity/1274

I think centering the image makes sense and both Google's and Samsung's gallery apps work like that. It might be worth passing the black background choice by a designer since black isn't a common color in Chromium (other than for movies).

[Elliott Sprehn]

On Sep 19, 2016 9:10 PM, "Philip Jägenstedt" <foo...@chromium.org> wrote:
>
> AFAIK there's no good reason for the difference, it's just an historical accident. ImageDocument is a pretty terrible image viewer, it could be made a lot better for large images in particular.
>
> Even if UA shadow roots are used, I think that'd be observable in that any attempt by the page to attach another shadow root would then fail. Pretty obscure, but I think it'd be worth investigating if these documents actually need to be considered same-origin with the page that embeds them.

Shadow DOM v1 doesn't allow author shadow roots on the body, so from the spec level we're fine. Shadow DOM v0 did, but I doubt the intersection of v0 and people messing with image documents is large. We already used the UA shadow root strategy for video IIRC.

>
> One might also consider using different documents for iframes and top-level frames.
>
>
> On Mon, Sep 19, 2016 at 8:42 PM Elliott Sprehn <esp...@chromium.org> wrote:
>>
>> Seems good to me! Note that the shape of the document body is actually required per spec, but we can use a UA shadow root here.
>>
>> Btw I think you're cross posting between public and private mailing lists. :)
>>
>>
>> On Sep 19, 2016 6:37 PM, <dfalc...@chromium.org> wrote:
>>>
>>> Hey all,
>>>
>>> As part of adding a new download manager UI to Chrome on Android, I've been investigating how standalone images & video are shown in tabs (e.g. for viewing files stored locally on your device or selecting "open image in new tab" from the context menu).  I tracked down the page generation to ImageDocument.cpp and MediaDocument.cpp, but was wondering about the inconsistency between the two: images are drawn starting in the top-left with a white background, while media is shown centered on the screen with a black background.
>>>
>>>
>>>

[Peter Kasting]

This is something I've wanted for a while:

https://bugs.chromium.org/p/chromium/issues/detail?id=118636

I don't know whether "on black" is best -- Firefox uses grey IIRC and that might be better.  More neutral contrast-wise.

It might also be nice to put some basic metadata (e.g. the image dimensions and possibly file size) on the page, but this is more debatable.  Just doing something at all would help.

PK

[Dan Alcantara]

Thanks all!  I'm a bit unfamiliar with how the process works here... If I were to look at addressing this (we're still debating a few options), would I have to send off an Intent of some sort (probably to experiment or implement)?

[Elliott Sprehn]

Assuming Chrome UX Is on board, I'd just send an Intent to Implement. That needs no approvals or anything, it's an FYI.

[Dan Alcantara]

Yep, Chrome UX's mocks kicked off this whole thread.  I'll sync up with them about styling (i.e. whether black makes sense).  Thanks!