|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
STR: 1. Load www.msn.com/en-us/news/good-news/ 2. Scroll down the page RESULT: Everything on the page is too big: text, images, space between headlines. Similarly: 1. Load http://www.msn.com/en-us/news/good-news/we-love-what-this-group-did-with-flowers-that-were-headed-for-the-trash/ar-BBDO2b9 2. Scroll down the page RESULT: The social media sharing buttons in the left column are too big and overlap the article text. Some of the article images are stretched too tall.
It seems they use rem for sizing. I think we have had some bugs for that...
Also, when I save the page locally, and strip all script, then this issue doesn't reproduce anymore. So there must be some dynamic change involves.
Anyway, seems to be font-size related, cc manish.
Emilio recently redid how rem works
What? I only fixed a bunch of bugs that https://github.com/servo/servo/pull/17041 left behind, the story is basically the same. I can take a look though, I guess...
So this is due to us updating the rem units from getDefaultComputedStyle. The fix is easy enough, I just need to construct a proper test-case...
Comment on attachment 8884368 [details] style: Move root font size handling outside of the cascade. https://reviewboard.mozilla.org/r/155290/#review160456 ::: servo/components/style/matching.rs:557 (Diff revision 1) > // The new root font-size has already been updated on the Device > // in properties::apply_declarations. This comment is no longer accurate and can be removed. ::: servo/components/style/matching.rs:562 (Diff revision 1) > // If the root font-size changed since last time, and something > // in the document did use rem units, ensure we recascade the > // entire tree. Maybe move this comment to just above the `if device.used_root_font_size()`, now that this block not only does this but also updates the root font-size on the Device.
Comment on attachment 8884369 [details] style: Avoid overriding the root font size from a getDefaultComputedStyle call. https://reviewboard.mozilla.org/r/155292/#review160458 I wonder if there is any other state that we update during a getDefaultComputedStyle that we shouldn't be... ::: servo/components/style/matching.rs:557 (Diff revision 1) > + // TODO(emilio): This should arguably be outside of the patch for > + // getComputedStyle/getDefaultComputedStyle, but it's unclear how to > + // do it without duplicating a bunch of code. Should it be "path" rather than "patch"? Otherwise I'm not sure I understand the comment.
Comment on attachment 8884370 [details] Bug 1379041: Reftest. https://reviewboard.mozilla.org/r/155294/#review160460
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/f0d0de89d505 Reftest. r=heycam
Servo bits landed in https://github.com/servo/servo/issues/17639.
Nightly 56 x64 20170709100223 @ Debian Testing (Linux 4.9.0-3-amd64, Radeon RX480) Verified fixed: * The social sharing buttons are now small as they should be on http://www.msn.com/en-us/news/good-news/we-love-what-this-group-did-with-flowers-that-were-headed-for-the-trash/ar-BBDO2b9 * http://www.msn.com/en-us/news/good-news/ is now like in Google Chrome