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

www.lazada.com.ph - Star rating UI elements are not properly displayed #28130

Open
cipriansv opened this issue Mar 21, 2019 · 3 comments
Open
Labels
browser-firefox engine-gecko The browser uses the Gecko rendering engine priority-normal severity-minor The site has a cosmetic issue. type-css Generic CSS issues type-js Generic JS issues
Milestone

Comments

@cipriansv
Copy link

URL: https://www.lazada.com.ph/products/bose-soundsport-free-wireless-headphones-midnight-blue-i156795182-s189140091.html?spm=a2o6y.10450891.0.0.3045c92ejLfoVR

Browser / Version: Firefox Nightly 68.0a1 (2019-03-20)
Operating System: Windows 10 Pro
Tested Another Browser: Yes

Problem type: Design is broken
Description: Star rating UI elements are not properly displayed
Steps to Reproduce:

  1. Navigate to https://www.lazada.com.ph/products/bose-soundsport-free-wireless-headphones-midnight-blue-i156795182-s189140091.html?spm=a2o6y.10450891.0.0.3045c92ejLfoVR
  2. Scroll down to the "Ratings & Reviews of Bose SoundSport Free Wireless Headphones - Midnight Blue" section.
  3. Observe the star rating UI elements.

Expected Behavior:
The star components of each UI element must be inline.

Actual Behavior:
The last component of each element is displayed on a different line.

Notes:

  1. The issue is not reproducible on Chrome 72.0.3626.121
  2. Screenshot attached.

Affected area:

<li>
	<div class="container-star progress-title" style="width:79.80000000000001px;height:15.96px">
		<img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
			<img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px" data-spm-anchor-id="a2o4l.pdp.ratings_reviews.i1.336972fdppqytV">
				<img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
					<img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
						<img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
						</div>
						<span class="progress-wrap">
							<div class="pdp-review-progress">
								<div class="bar bg"></div>
								<div class="bar fg" style="width:80%"></div>
							</div>
						</span>
						<span class="percent">8</span>
					</li>

Watchers:
@softvision-oana-arbuzov
@softvision-sergiulogigan
@cipriansv

sv;
Screenshot Description

Browser Configuration
  • None

From webcompat.com with ❤️

@webcompat-bot webcompat-bot added this to the needstriage milestone Mar 21, 2019
@cipriansv cipriansv changed the title www.lazada.com.ph - design is broken www.lazada.com.ph - Star rating UI elements are not properly displayed Mar 21, 2019
@cipriansv cipriansv added the severity-minor The site has a cosmetic issue. label Mar 21, 2019
@cipriansv cipriansv modified the milestones: needstriage, needsdiagnosis Mar 21, 2019
@karlcow karlcow self-assigned this Mar 25, 2019
@karlcow
Copy link
Member

karlcow commented Mar 25, 2019

<li>
  <div class="container-star progress-title" style="width:79.80000000000001px;height:15.96px">
    <img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
    <img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
    <img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
    <img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
    <img class="star" src="//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png" style="width:15.96px;height:15.96px">
  </div>
  <span class="progress-wrap">
    <div class="pdp-review-progress">
      <div class="bar bg"></div>
      <div class="bar fg" style="width:80%"></div>
    </div>
  </span>
  <span class="percent">8</span>
</li>

The size seems to be dynamically adjusted given the values into each style attribute.

The width of the wrapper is 79.80000000000001px
The width of each img is 15.96px

15.96 * 5 = 79.8px

which makes it really tight and subject to any pixels rounding issue.

for example if I set it to be 79.83px; this is working. And If we want to be more precise.

  • 79.82499313354491px FAIL
  • 79.82499313354492px working

which makes a diffference of 0.02499313354px in between Chrome and Firefox.

This is computed here:

          o(t, [
            {
              key: "select",
              value: function(e) {
                var t = this.props.onSelect;
                t && t(e);
              }
            },
            {
              key: "render",
              value: function() {
                var e = this,
                  t = this.props,
                  a = t.size,
                  r = void 0 === a ? 12 : a,
                  o = t.score,
                  n = void 0 === o ? 0 : o,
                  l = t.className,
                  s = void 0 === l ? "" : l,
                  c = [
                    "//laz-img-cdn.alicdn.com/tfs/TB15K7RdOqAXuNjy1XdXXaYcVXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB17MwRdOqAXuNjy1XdXXaYcVXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB16MwRdOqAXuNjy1XdXXaYcVXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB16gwRdOqAXuNjy1XdXXaYcVXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB13svEgfDH8KJjy1XcXXcpdXXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB14IvEgfDH8KJjy1XcXXcpdXXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB14buYglfH8KJjy1XbXXbLdXXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB19svEgfDH8KJjy1XcXXcpdXXa-64-64.png",
                    "//laz-img-cdn.alicdn.com/tfs/TB14HuYglfH8KJjy1XbXXbLdXXa-64-64.png"
                  ],
                  u =
                    "//laz-img-cdn.alicdn.com/tfs/TB18ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png",
                  d = ("" + n).split("."),
                  f = u,
                  p = parseInt(d[0]),
                  h = parseInt((d[1] && d[1].charAt(0)) || 0),
                  m = 1.33 * r;
                return i.default.createElement(
                  "div",
                  {
                    className: "container-star " + s,
                    style: {
                      width: 5 * m + "px",
                      height: m + "px"
                    }
                  },
                  [1, 2, 3, 4, 5].map(function(t, a) {
                    return (
                      (f =
                        p >= t
                          ? "//laz-img-cdn.alicdn.com/tfs/TB19ZvEgfDH8KJjy1XcXXcpdXXa-64-64.png"
                          : p === t - 1 && h > 0
                          ? c[h - 1]
                          : u),
                      i.default.createElement("img", {
                        key: a,
                        className: "star",
                        src: f,
                        style: {
                          width: m + "px",
                          height: m + "px"
                        },
                        onClick: function() {
                          e.select(t);
                        }
                      })
                    );
                  })
                );
              }
            }
          ]),
          t
        );

t = {score:5, size:12}
a = 12
r = 12

so m = 1.33 * r so 15.96

width: 5 * m so 79.8

which I guess for chrome with the rounding issues gives 79.80000000000001px

The computed sizes as displayed in the layout with getBoundingClientRect()

Chrome

  • stars: width: 15.953125, height: 15.953125
  • container: width: 79.796875, height: 15.953125

Firefox

  • stars: width: 15.966659545898438, height: 15.9666748046875
  • container: width: 79.80000305175781, height: 15.9666748046875

I guess the main difference is here in the layout.

@dholbert Can we consider this acceptable differences? Of should it be the same values?

TO NOTE: If they remove the width constraint on the container. Everything is working.
OR: They could change the display value of the container, and it would not wrap anymore.

/* pc-mod.css | https://laz-g-cdn.alicdn.com/lzdfe/pdp-modules/1.0.13/pc-mod.css */

.pdp-mod-review .mod-rating .detail li .progress-title {
  /* display: inline-block; */
  display: inline-flex;
}

I'm pushing to needscontact.

@karlcow karlcow added type-css Generic CSS issues type-js Generic JS issues labels Mar 25, 2019
@karlcow karlcow modified the milestones: needsdiagnosis, needscontact Mar 25, 2019
@karlcow karlcow removed their assignment Mar 25, 2019
@dholbert
Copy link

dholbert commented Mar 25, 2019

Yeah, Firefox's layout engine doesn't support CSS sizes with a fine enough precision to represent exact hundredths-of-a-pixel. We use a fixed point representation which rounds to the nearest 60th of a pixel.

So, if the site has width: 15.96px, then for layout purposes, we round that to the nearest 60th-of-a-pixel which is 15.966667px (which introduces a rounding error of .006667px) -- and with 5 of these rounded-up widths, we would end up with a collective .03333px (repeating) of rounding error, which is why we end up barely overshooting the width of the container.

We have https://bugzilla.mozilla.org/show_bug.cgi?id=265084 filed on thinking about changing our representation, but it's unlikely to happen soon. So for now, we should recommend that sites avoid depending on fine-tuned barely-fitting layout arithmetic with hardcoded hundredth-of-a-pixel sizes.

@dholbert
Copy link

dholbert commented Mar 25, 2019

And in particular: if a site wants to precisely divide up a width into N equal chunks, the most reliable way to do that is using CSS Flexbox. E.g. if they restyle the wrapper here to have display: inline-flex, then this Just Works, as karl observed above (because, by default, flexbox will detect the tiny overflow of all the desired sizes (from the rounding error in our representation of 15.96px), and it'll shrink each item fairly (and negligibly in this case) to fit.

@miketaylr miketaylr added the engine-gecko The browser uses the Gecko rendering engine label Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser-firefox engine-gecko The browser uses the Gecko rendering engine priority-normal severity-minor The site has a cosmetic issue. type-css Generic CSS issues type-js Generic JS issues
Projects
None yet
Development

No branches or pull requests

5 participants