Skip to content

/ servo

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

Write a reftest for the background attribute on the body element #6992

Closed
wants to merge 2 commits into from
Closed

Write a reftest for the background attribute on the body element #6992

wants to merge 2 commits into from

Conversation

@sgmenda
Copy link
Contributor

sgmenda commented Aug 5, 2015

Testing the background in a basic way. #6838

Review on Reviewable
s142857
added test for background tag, none
Loading status checks…
6fa8cec
@highfive
Copy link

highfive commented Aug 5, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @jdm (or someone else) soon.
@jdm
Copy link
Member

jdm commented Aug 5, 2015

Thanks @s142857! There are a couple issues here:

  • none of the tests are using the <body background="foo"> feature that we want to check
  • the tests are still in test/ref so they won't be found.

We will want to create a branch new pair of test files in tests/wpt/mozilla/tests/css, one of which should use <body background> and the other (the _ref.html file) should use a CSS background-image on the body element. Does that make sense?
@jdm
Copy link
Member

jdm commented Aug 5, 2015

The changes to tests/ref/background_image_position_a.html and tests/ref/background_none_a.html should be reverted, as well as the removal of tests/ref/400x400_green.png. We're also missing the new files.
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 5, 2015

i added them check added commit
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 5, 2015

i did not change the files, i actually copied them to wpt then changed then deleted the files
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 5, 2015

if you are talking about addition of link, it is harmless and makes it relatively easier in the future
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 5, 2015

i could remove the links if you want me to
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 5, 2015

thanks buddy
@jdm
Copy link
Member

jdm commented Aug 6, 2015

400x400_green.png was moved from test/ref, which will break any reftests there that use it. Please do remove the new <link> changes in the old tests, too.
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 6, 2015

I copied it, not moved, it i still exists in tests/ref

On Thu, Aug 6, 2015 at 11:05 AM, Josh Matthews notifications@github.com
wrote:

400x400_green.png was moved from test/ref, which will break any reftests
there that use it. Please do remove the new changes in the old
tests, too.

—
Reply to this email directly or view it on GitHub
#6992 (comment).

Sanketh
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 6, 2015

ok, I will change the path

On Thu, Aug 6, 2015 at 11:12 AM, Sanketh Menda mendasanketh@gmail.com
wrote:

I copied it, not moved, it i still exists in tests/ref

On Thu, Aug 6, 2015 at 11:05 AM, Josh Matthews notifications@github.com
wrote:

400x400_green.png was moved from test/ref, which will break any reftests
there that use it. Please do remove the new changes in the old
tests, too.

—
Reply to this email directly or view it on GitHub
#6992 (comment).

Sanketh

Sanketh
@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 6, 2015

I'll place the 400_400_green.png back in its place and make a new file
(Like red_400_400.png) for wpt tests, there are more 16million possibilites
right!!

Give me 5hrs to do the needed, I need to go home and do it, I'm in school.

On Thu, Aug 6, 2015 at 11:13 AM, Sanketh Menda mendasanketh@gmail.com
wrote:

ok, I will change the path

On Thu, Aug 6, 2015 at 11:12 AM, Sanketh Menda mendasanketh@gmail.com
wrote:

I copied it, not moved, it i still exists in tests/ref

On Thu, Aug 6, 2015 at 11:05 AM, Josh Matthews notifications@github.com
wrote:

400x400_green.png was moved from test/ref, which will break any reftests
there that use it. Please do remove the new changes in the old
tests, too.

—
Reply to this email directly or view it on GitHub
#6992 (comment).

Sanketh

Sanketh

Sanketh
@jdm jdm added S-needs-squash and removed S-awaiting-review labels Aug 6, 2015
@sgmenda sgmenda force-pushed the sgmenda:master branch from ac89390 to 13e8209 Aug 6, 2015
@sgmenda sgmenda force-pushed the sgmenda:master branch from 13e8209 to 4e1a8e5 Aug 6, 2015
s142857
Add a reftest for <body background>. Fixes #6838
Loading status checks…
039f951 
mod manifest
@sgmenda sgmenda force-pushed the sgmenda:master branch from 4e1a8e5 to 039f951 Aug 6, 2015
@jdm jdm mentioned this pull request Aug 6, 2015
Merged
@jdm jdm closed this Aug 6, 2015
Manishearth added a commit to Manishearth/servo that referenced this pull request Aug 6, 2015
@Manishearth
Rollup merge of servo#7039 - jdm:squashed-6992, r=jdm
2114ffd 
Add a reftest for <body background>. Fixes servo#6838

Rebased from servo#6992.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7039)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
S-awaiting-review S-needs-squash
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@sgmenda @highfive @jdm
You can’t perform that action at this time.