Closed Bug 1275966 Opened 3 years ago Closed 3 years ago

background-repeat:space doesn't fit an image that fits exactly

Categories

(Core :: Web Painting, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: dbaron, Assigned: ethlin)

References

Details

(Keywords: dev-doc-complete)

Attachments

(4 files, 2 obsolete files)

background-repeat: space doesn't fit an image that fits exactly.

In the attached testcase, you should see the image tiled 4 times horizontally (with spacing) and 2 times vertically (without spacing).

Firefox incorrectly shows only 1 vertical tile.  Chromium and Edge get it correct.
Flags: needinfo?(ethlin)
Attached file testcase
Attachment #8756918 - Attachment mime type: text/plain → text/html
Attached patch Part1. Fix space checking. (obsolete) — Splinter Review
I will write a test case for it.
Assignee: nobody → ethlin
Status: NEW → ASSIGNED
Flags: needinfo?(ethlin)
Attachment #8757102 - Attachment is obsolete: true
Attached patch fix sapce checking (obsolete) — Splinter Review
I'd like the repeat size to keep its meaning. You could add a bool outparam to ComputeSpacedRepeatSize that determines whether we should repeat in that dimension, and then you set the ExtendMode to the right value based on that, and also check the extend mode in nsLayoutUtils::DrawBackgroundImage.
(In reply to Markus Stange [:mstange] from comment #4)
> I'd like the repeat size to keep its meaning. You could add a bool outparam
> to ComputeSpacedRepeatSize that determines whether we should repeat in that
> dimension, and then you set the ExtendMode to the right value based on that,
> and also check the extend mode in nsLayoutUtils::DrawBackgroundImage.

Okay, that wip is just one possible way to fix it. I will add a bool outparam for that.
Add a bool parameter to determine whether the image is repeated.
Attachment #8757111 - Attachment is obsolete: true
Attachment #8757631 - Flags: review?(mstange)
I added two test cases to check if space works fine while the image x or y fits exactly.
Attachment #8757632 - Flags: review?(mstange)
Attachment #8757631 - Flags: review?(mstange) → review+
Comment on attachment 8757632 [details] [diff] [review]
Part2. Add reftests.

Review of attachment 8757632 [details] [diff] [review]:
-----------------------------------------------------------------

Did you forget to refresh this patch before you attached it? The tests don't look like they're testing this bug, and I think both reftests are the same.
Attachment #8757632 - Flags: review?(mstange)
(In reply to Markus Stange [:mstange] from comment #8)
> Comment on attachment 8757632 [details] [diff] [review]
> Part2. Add reftests.
> 
> Review of attachment 8757632 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Did you forget to refresh this patch before you attached it? The tests don't
> look like they're testing this bug, and I think both reftests are the same.

I think the patch should be correct. For "background-repeat-space-9-ref.html", the background position size is 106x96, and the image is 32x32, so the image fix exactly on Y direction. And "background-repeat-space-10-ref.html" size is 96x106, which is for testing X direction. I will upload the snapshot of the test.
Oh, nevermind then.
Attachment #8757632 - Flags: review+
Attached image result.png
This is the before/after result of background-repeat-space-10-ref.html.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25fb10028068
Part 1. Fix wrong rendering of background-repeat:space. r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/005a02a60fbd
Part 2. Add reftests for background-repeat: space. r=mstange
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/25fb10028068
https://hg.mozilla.org/mozilla-central/rev/005a02a60fbd
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Blocks: 1277375
No longer blocks: 1277375
Depends on: 1277375
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.