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

Table cell that has multiple rows shows diff cell colors incorrectly #214

Closed
rlopez133 opened this issue Jun 22, 2015 · 33 comments
Closed
Assignees
Labels

Comments

@rlopez133
Copy link

When I have a cell that spans multiple rows, its showing diff cell colors for each row. This does not happen in alpha7 but in dev.

.Hardware Details
|====
^|Hardware ^|Specifications

.5+|Provisioning node (Red Hat Enterprise Linux OpenStack Installer)
[1 x Dell PowerEdge R610 ]| Red Hat Enterprise Linux 7.1 x86_64
kernel 3.10.0-229.el7.x86_64 
| 4 x Broadcom Gigabit Ethernet 5709C |
2 Socket, 12 Core (24 cores) 
Intel(R) Xeon(R) CPU X5650 @ 2.67 Ghz 
| 48 GB of memory, DDR3 8GB @ 1333 MHz |
2 x 500 GB SAS internal disk drives 
|====

Latest alpha output (It should have this behavior).
selection_225

dev output:

selection_226

Doesn't really show up well here, but the color used in the alpha for diff colors per row looks better to me than what is now used in dev.

@mojavelinux
Copy link
Member

I wonder if this is a bug that was inherited by the upgrade to Prawn Table. I'll investigate to see if I can figure out the origin of the issue.

@mojavelinux
Copy link
Member

I think perhaps your dependencies are out of sync because I'm not seeing this in dev. Could you retest once 1.5.0.alpha.8 is out?

@mojavelinux
Copy link
Member

alpha.8 is out. Can you retest?

@mojavelinux
Copy link
Member

Here's what I get with the latest dev using your theme.

table-1

@mojavelinux mojavelinux added this to the support milestone Jun 26, 2015
@rlopez133
Copy link
Author

Dan,

Working on possibly getting an env you can look at from my end but in the meantime I wanted to tell you what I'm doing on my end see if you can reproduce.

  1. Installation of RHEL 7.1
  2. Installed RVM using instructions provided
  3. git clone the asciidoctor-pdf repository
  4. Install bundler using gem
  5. cd into asciidoctor-pdf-dev directory
  6. use bundle to install dependencies.

This is what is installed and versions. I figured I'd include this to see if maybe there is some discrepancy in versions that is causing issue on my end that you are not seeing.

From within the asciidoctor-pdf-dev directory, 

# bundle
Don't run Bundler as root. Bundler can ask for sudo if it is needed, and installing your bundle as root will break this
application for all non-root users on this machine.
Fetching gem metadata from https://rubygems.org/..........
Fetching version metadata from https://rubygems.org/..
Resolving dependencies...
Using rake 10.4.2
Installing Ascii85 1.0.2
Installing addressable 2.3.8
Installing afm 0.2.2
Installing asciidoctor 1.5.2
Installing pdf-core 0.5.1
Installing ttfunk 1.4.0
Installing prawn 2.0.1
Installing prawn-icon 0.6.4
Installing css_parser 1.3.6
Installing prawn-svg 0.21.0
Installing prawn-table 0.2.1
Installing hashery 2.1.1
Installing ruby-rc4 0.1.5
Installing pdf-reader 1.3.3
Installing prawn-templates 0.0.3
Installing safe_yaml 1.0.4
Installing thread_safe 0.3.5
Installing polyglot 0.3.5
Installing treetop 1.5.3
Using asciidoctor-pdf 1.5.0.dev from source at .
Using bundler 1.10.5
Bundle complete! 2 Gemfile dependencies, 22 gems now installed.
Use `bundle show [gemname]` to see where a bundled gem is installed.
Post-install message from pdf-reader:

@mojavelinux
Copy link
Member

Thanks @rlopez133!

Don't run Bundler as root.

This tells me that you're not actually using RVM. Can you run the following command and paste the output?

$ rvm current

I expect it to say something like:

ruby-2.2.1

If instead it says "system", then you're not actually using RVM.

@mojavelinux
Copy link
Member

Install bundler using gem

This also tells me you're not using RVM because bundler comes with RVM preinstalled.

@mojavelinux
Copy link
Member

The best way to reproduce our environments is to use Docker. Perhaps we can use the equivalent version of CentOS instead of RHEL as I don't have RHEL. Here's the CentOS Docker image:

https://registry.hub.docker.com/_/centos/

@rlopez133
Copy link
Author

As root user,
# rvm current
ruby-2.2.1

@mojavelinux
Copy link
Member

That's good news! I see that the message from bundler is because "As root user" :)

It's probably best if you create a user, but I don't think that's going to affect the results, just a general recommendation.

@rlopez133
Copy link
Author

No argument there. :)

@mojavelinux
Copy link
Member

How are you running asciidoctor-pdf from dev? Are you running it like this?

./bin/asciidoctor-pdf

@rlopez133
Copy link
Author

ruby /root/asciidoctor-pdf-dev/asciidoctor-pdf/bin/asciidoctor-pdf -a pdf-style=asciidoctor <name-of-file>.adoc

@mojavelinux
Copy link
Member

You don't need the "ruby" part, but that probably won't affect the results.

@mojavelinux
Copy link
Member

I'm going to attempt this on CentOS 7.

It would really help if you pushed the latest theme and sample files to the refarch-asciidoc repository and made sure one of the files in that repository demonstrates the issue. That way, I can clone it from inside Docker and (in theory) get the same results.

@mojavelinux
Copy link
Member

...or create a branch if you need to preserve master.

@rlopez133
Copy link
Author

OK, let me see what I can do. 👍

@mojavelinux
Copy link
Member

I misspoke. RVM doesn't come with bundler by default. That's very strange. I thought it did. Okay, so gem install bundler is required. Good to know.

@mojavelinux
Copy link
Member

I just setup a fresh install on CentOS 7.1 using Docker and my output is exactly the same as the last screenshot I posted. That means that, if there is a difference, it's coming from either your documents or how you are running Asciidoctor PDF.

I'll document the steps I used to setup Docker so that you can reproduce. If you follow the exact same steps and still see this problem, then we'll at least have isolated the differences.

@rlopez133
Copy link
Author

sounds like a plan! Tomorrow I'll update my repo with the theme. As for the whole document, not sure I'll be able to post that yet, but I can definitely maybe just run a specific chapter and see if I can reproduce. I'll update this tomm, when I have some more info.

@mojavelinux
Copy link
Member

mojavelinux commented Jun 30, 2015 via email

@mojavelinux
Copy link
Member

Here are the steps I followed to get a test setup for Asciidoctor PDF on CentOS 7.1 using Docker:

$ docker pull centos:7
$ docker run --privileged=true -v /tmp:/tmp -i -t centos:7 /bin/bash
# yum install -y curl sudo vim git passwd
# useradd -m user
# visudo # change entry that starts with %wheel to %user
# passwd user # change password for user account
# su - user
$ curl -sSL https://rvm.io/mpapis.asc | gpg2 --import -
$ curl -sSL https://get.rvm.io | bash -s stable
$ source ~/.profile
$ sudo ls # rvm install requires cached sudo credentials to install required rpms
$ rvm install 2.2
$ rvm use 2.2@global
$ gem install bundler
$ rvm use 2.2@asciidoctor-pdf-testing --create
$ git clone https://github.com/asciidoctor/asciidoctor-pdf
$ cd asciidoctor-pdf
$ bundle
$ ./bin/asciidoctor-pdf -D /tmp README.adoc

What's left is to grab the rh-theme.yml and sample AsciiDoc files from another repo and run them to see if we get the same results.

Note that I mounted the /tmp directory inside the Docker container to the host /tmp directory so you have an easy way to pass files back and forth. Alternatively, you could use scp.

@mojavelinux
Copy link
Member

I can duplicate the problem!!

Thanks for sticking with me on this one, @rlopez133. I'm now seeing the problem. I haven't yet been able to determine a solution, but at least I have something I can probe. Stay tuned.

@mojavelinux
Copy link
Member

I was right in that it depends on the content somehow. If I put only the table in a document and convert it, it looks fine. When it's in the middle of the document, I'm able to reproduce the bug.

@mojavelinux
Copy link
Member

It happens when the table is inside of a section. Now I need to figure out what a section has to do with the background on a table cell.

@mojavelinux
Copy link
Member

Wow, I feel like kind of a moron now that I've discovered what's going on. In my attempt to optimize the code, I turned off the background color on the table if the color is white and the parent is a section (meaning it's not nested inside a block). (Of course, I forgot about the code if the parent is a document).

See https://github.com/asciidoctor/asciidoctor-pdf/blob/master/lib/asciidoctor-pdf/converter.rb#L1052-L1057

The reason this was hard to reproduce was because it only manifests when the table is in a certain part of the document. That's why the reproducible test case (or test document) is much more valuable than a screenshot ;) If we can move forward working on sample documents, it's going to save us a ton of time.

Now...to fix the issue.

@mojavelinux
Copy link
Member

...the screenshots are still helpful to explain what to look for, but the sample document is essential for debugging.

@mojavelinux
Copy link
Member

We now understand how Prawn shades cells that span rows. It first shades the rows in alternating colors, then it covers over it with the color of the cell spanning those rows. We saw bleed-through because the odd row background color was nil (as a result of the table background color being nil).

@mojavelinux mojavelinux self-assigned this Jun 30, 2015
mojavelinux added a commit that referenced this issue Jun 30, 2015
resolves #214 use table background color, even if white
@mojavelinux
Copy link
Member

Give master a try now and see if you get the results you expect. I do recommend setting the background color on the table, but you should no longer have to set the background color of the row you want to be white.

Btw, here's a shorthand way to set the color of the table background color:

table:
  background_color: $page_background_color

@mojavelinux mojavelinux modified the milestones: v1.5.0, support Jun 30, 2015
@mojavelinux
Copy link
Member

Hopefully, we can start moving forward again :)

@mojavelinux
Copy link
Member

The post-mortem of this issue is that the transparent cell background was a bug I caused, but a bug in prawn-table (prawnpdf/prawn-table#45) was throwing me off the scent. I've addressed both situations.

@rlopez133
Copy link
Author

Just tested it, and confirmed fixed! :) Glad you were able to find this one, it was definitely a pain! lol

@mojavelinux
Copy link
Member

\o/

Sometimes, the programs like to test our commitment :) We're committed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants