Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve values for rgb colors #1434

Conversation

u1f992
Copy link

@u1f992 u1f992 commented Dec 21, 2024

fix: #1432

setProperty(prop, value)で文字列が維持されないvalueをCSS変数に退避する方針に切り替えて実装しました。要素のCSSの編集はすべてsetPropertyで行っているので、widthheightが繰り返し設定されるような事態は避けられているはずです。誤差は丸められるままにしておきたい点については、別途考慮しています。

CSS変数名・style要素IDは指示いただいたものに直すつもりです。キーはひとまずbase64になっています。

現状気づいている微妙な挙動として、デフォルト値を省略した場合にも退避されてしまうようです。counter-incrementなどで目立ちます。

image

Copy link

vercel bot commented Dec 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vivliostyle ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 22, 2024 0:24am

@MurakamiShinyu
Copy link
Member

PRありがとうございます。
ざっと見ましたが、解決しようとする問題やそのニーズに対して、追加される処理で複雑さが増すことのコストが大きすぎはしないかと懸念します。
もともとの問題(#1432)は、rgb() の精度が失われるということでしたが、それによる実害はどのようなものがありますか?

また、問題はChromiumでのプロパティ値のシリアライズにあるのですが、Chromiumにissue登録して、その早期解決を促すというのでは、解決まで時間がかかりすぎて待てないということでしょうか?

@u1f992
Copy link
Author

u1f992 commented Dec 22, 2024

すみません、コメントに気づかず追加のpushを行いました。村上さんが提示された懸念について根本的な解決に至るものではありません。

もともとの問題(#1432)は、rgb()の精度が失われるということでしたが、それによる実害はどのようなものがありますか?

IssueおよびPRは、直面している問題への対処というよりも、PDF出力能力を損なっていることは惜しいというモチベーションによるものです。
画面表示においては実害はほとんどありません。
PDFを印刷に使用するケースでは、色表現に差が出る可能性がありますが、いずれにしても今すぐ具体例を提示できるほどの問題はありません。

また、問題はChromiumでのプロパティ値のシリアライズにあるのですが、Chromiumにissue登録して、その早期解決を促すというのでは、解決まで時間がかかりすぎて待てないということでしょうか?

Chromiumの修正を求めるよりも、Vivliostyleで回避策を実装する方がはるかに現実的であると考えています。
インラインスタイルにおいて実数を含むrgb()が8ビット整数のレガシー記法に正規化される件については、ChromiumだけでなくFirefoxでも同様の挙動になっています。これは互換性を重視した暗黙の合意による挙動であると推測しており、Webブラウザが主に画面表示を目的としていることを考えれば、合理的な妥協だとも理解しています。
既存のJavaScriptコードの互換性を考慮すると、この件が問題として認識されるかどうかについて確証がありません。


複雑さが増すことはおっしゃる通りで、実装力と理解不足により、私からパッチを提案するのは難しい問題かもしれません。
なお、「一時要素に追加して一致しない」かつ「rgb( ... )を含む」場合に限り退避するという、もう少しシンプルかもしれない案があります。

@u1f992
Copy link
Author

u1f992 commented Dec 23, 2024

村上さんのコメントを受けて内部を読み進めるうちに、これまで目を付けていた周辺に手を加えるだけでは、むやみに複雑な処理を追加する結果にしかならないことがわかってきました……。

CSS変数に退避する発想は残すにせよ、より正当に対処するためには、PREPROCESS_ELEMENT_STYLEに対するHookとして処理を追加する必要があるということでしょうか。

this.preprocessElementStyle(computedStyle);
this.applyComputedStyles(result, computedStyle);

@u1f992
Copy link
Author

u1f992 commented Dec 23, 2024

再度Issueに差し戻します。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preserve fractional values for colors
2 participants