w3c / i18n-glossary

Definitions of terms used in W3C Internationalization documents.
https://w3c.github.io/i18n-glossary/
4 stars 4 forks source link

auto-sizing circumgraph image in chrome #38

Closed aphillips closed 9 months ago

aphillips commented 1 year ago

I'm seeing poor spacing when I view the ReSpec ED in Chrome with character images. I couldn't get it to work correctly for the FFFC image and I notice that circumgraph looks like this:

image

Chrome is Version 111.0.5563.112 (Official Build) (64-bit) on Windows 11.

Firefox looks fine. @r12a, any thoughts?

r12a commented 1 year ago

This appears to be because something is adding height and width attributes to the img element in Chrome. The source of the Chrome rendered document says:

<img src="img/circumgraph.svg" alt="କୋ" style="height:1.2rem;" height="150" width="261">

Those attributes are not in the source when displayed by Firefox.

Safari also has the added attributes, but they seem to be calculated based on the result of the style attribute, rather than on the original size of the image (so it looks ok).

The attributes don't appear to be added in my own documents, so i'm inclined to suspect that this is something Respec is doing.

A workaround might be to specify the width as well as the height in the style attribute, or to use height and width attributes to specify the dimensions. You can probably get a figure for the setting from the inspector (Computed).

r12a commented 1 year ago

It may be worth looking into whether there's a way of telling Respec not to do this, or requesting one if there isn't.

aphillips commented 1 year ago

I did try adding the height and width attributes explicitly, but that didn't work!

r12a commented 1 year ago

Strange. Worked for me. I tried adding both in attributes and in styling. Note that attributes can only carry digits representing pixel values.

aphillips commented 1 year ago

I can reproduce this as an error in Chrome when adding height="150" width="261" (and reversing the order of the attributes). It also does not work when I use just width or just height. When I put the size as height="20rem" width="34.8rem", though, it magically works and the size is set correctly (and the rem sizes or em sizes have an effect on what size the graphic is shown as).

As you note, this is not legal HTML???

aphillips commented 1 year ago

PS> The rem sizing also works in FF. Editing the height or width in the console has the right visual effect.

image

... edit to double width...:

image

xfq commented 1 year ago

There seems to be a related bug in ReSpec: https://github.com/w3c/respec/issues/3435

I tried adding attributes to the HTML and that seemed to do the trick, I'll raise a PR so we can see the preview.

r12a commented 1 year ago

This now appears to be fixed. See https://w3c.github.io/i18n-glossary/#dfn-circumgraph in Chrome. Thanks to @sidvishnoi .

Reopen if there's more to say

r12a commented 1 year ago

Btw, note that i think it's only fixed for SVG images. If we use the large PNGs in https://github.com/r12a/c to represent characters, we'll probably still need to manually change the attribute dimensions, rather than just fix it by changing a line in the style sheet.

sidvishnoi commented 1 year ago

@r12a If it causes issues, I think we can add an opt-out mechanism (something like data-respec-ignore or a respecConfig option), or remove this feature altogether.

r12a commented 1 year ago

@sidvishnoi i'd welcome at least an opt out. I think i'd prefer not to add those attributes at all, but i don't know what removing them completely would do to backwards compatibility. For me, such presentation-related metadata shouldn't be in attributes, and should only be in the element tag for truly unique requirements.

I think that, for us, a respecConfig option would be far more useful than having to add a (long) bit of meta-data to every instance. For others, it may be better to have the option to use either(?)

r12a commented 1 year ago

@sidvishnoi any progress on this? I wanted to include an inline png image in a document yesterday, but had to give up. (The original size was much larger than the line, and i wasn't able to reduce the size using CSS.)

sidvishnoi commented 1 year ago

Haven't got opportunity yet. Removing that functionality would be a breaking change, and adding another option would be maintenance++ (can't remove it easily in future), so I'm not sure if ReSpec should do either.

As a quick fix, can you add a respecConfig.preProcess function to add width/height attribute so ReSpec ignores those images, and remove those attributes in a postProcess function? ReSpec ignores on these rules:

":not(picture)>img:not([width]):not([height]):not([srcset])"

If not feasible, I might make the breaking change.

r12a commented 1 year ago

If not feasible, I might make the breaking change.

Or provide for a flag to be set in the settings at the top of the page, such as insertHeightWidthAttributes = false ?

r12a commented 11 months ago

Note that Respec no longer has code that automatically adds height and width attributes to img elements.

See https://github.com/w3c/respec/pull/4432

Thanks, @sidvishnoi !

aphillips commented 9 months ago

@xfq: is this fixed, then? Can we close this? Don't forget to delete your branch if it is no longer needed.

xfq commented 9 months ago

I personally prefer to add height and width to all images to reduce layout shifts, but I don't feel strongly to object and will close this issue.

r12a commented 9 months ago

You should still be able to add height and width information to individual images, but respec will no longer force that upon everyone, and will allow flexibility in how it is done which is particularly useful when dealing with sets of images that need to the same dimensions.