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

Ignore pageColors when the background/foreground is identical (PR 14874 follow-up) #14887

Merged
merged 2 commits into from
May 8, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

If the computed background/foreground colors are identical, the canvas would be rendered mostly blank with only images visible. Hence it seems reasonable to also ignore the pageColors-option in this case.

Also, the patch tries to briefly outline the various cases in which we ignore the pageColors-option in a comment.

…874 follow-up)

Given that the new API-option is an Object named `pageColors`, with `background`/`foreground` keys, it occurred to me that it'd be slightly more consistent if the options/preferences names fully reflected that.
@Snuffleupagus Snuffleupagus force-pushed the pageColors-followup branch from 66bceed to d8d7bc0 Compare May 8, 2022 09:15
…4874 follow-up)

If the computed background/foreground colors are identical, the `canvas` would be rendered mostly blank with only images visible. Hence it seems reasonable to also ignore the `pageColors`-option in this case.

Also, the patch tries to *briefly* outline the various cases in which we ignore the `pageColors`-option in a comment.
@Snuffleupagus Snuffleupagus changed the title Ignore pageColors when the background/foreground is identical (14874 follow-up) Ignore pageColors when the background/foreground is identical (PR 14874 follow-up) May 8, 2022
@Snuffleupagus Snuffleupagus force-pushed the pageColors-followup branch from d8d7bc0 to 472a1f9 Compare May 8, 2022 09:41
Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it wasn't possible to have the same values... but who knows.

@Snuffleupagus
Copy link
Collaborator Author

I thought it wasn't possible to have the same values... but who knows.

I was trying to protect against the user manually specifying e.g. pageColors: { background: white, foreground: white }, or something equally pointless.

@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented May 8, 2022

From: Bot.io (Linux m4)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/6f536a38ab70364/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 8, 2022

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/95511893920df7b/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 8, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/6f536a38ab70364/output.txt

Total script time: 26.42 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 4
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/6f536a38ab70364/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

pdfjsbot commented May 8, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/95511893920df7b/output.txt

Total script time: 31.28 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@Snuffleupagus Snuffleupagus merged commit 3d9b2c9 into mozilla:master May 8, 2022
@Snuffleupagus Snuffleupagus deleted the pageColors-followup branch May 8, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants