Fix nested tags, make tests pass #25
Conversation
Gemfile
Outdated
| @@ -18,7 +18,7 @@ group :test do | |||
| gem "capybara" | |||
| gem "evergreen" | |||
| gem "capybara-webkit" | |||
| gem 'selenium-webdriver' | |||
| gem 'selenium-webdriver', '2.53.4' | |||
There was a problem hiding this comment.
via this (https://docs.travis-ci.com/user/firefox/) you should be able to select the version of firefox that is compatible with recent versions of selenium-webdriver
There was a problem hiding this comment.
I tried using the latest Firefox but that also seems to be broken with the latest selenium driver for Ruby. I can look more into this next week probably.
OTOH I can also revert to the previous commit which uses selenium-webdriver 2.53.4, In fact I will prefer this b/c on my system I have an older Firefox version (for now).
atodorov
force-pushed
the
fix_nested_tags_tests
branch
from
580f472
to
0f731b8
Compare
|
@jhilden I've tried the latest firefox (53.0) but it failed in Travis. Any ideas besides sticking to an older selenium-webdriver so we can move forward with this ? |
The previous implementation pushed with commit 83c7f8f fails the tests and I believe is wrong. As far as I can understand .textNodes() returns a list of all text nodes of the current element, which are then stripped of the --i18n.key-- annotations. The trouble is that .textNodes() uses .contents() which only searches the immediate children in the DOM tree. Thus is we have nested HTML tags the text nodes of these nested tags are not returned and the i18n annotations are not removed. This is indeed what is causing the existing tests to fail: https://travis-ci.org/railslove/i18n_viz/jobs/158501488 This new implementation doesn't use .textNodes() and instead recurses into child nodes on its own.
atodorov
force-pushed
the
fix_nested_tags_tests
branch
from
0c28d99
to
43e7724
Compare
|
ok, I think I got it working now. Apparently we now need to install the thing called |
@jhilden so this is an attempt to fix the mess I've created in #19 and you were so kind enough to merge without proper testing.
The first commit fixes the version of selenium-webdriver b/c newer versions don't work with older Firefox (which we have in Travis).
The second commit provides new implementation which keeps nested HTML tags:
ATM I don't have access to the Rails site which I used to integrate i18n_viz with (was a contract work and I don't have another similar Rails project). The tests now PASS and the logic above sounds like it's the right thing to do. I don't really remember what I was thinking in my previous attempt at this and we don't have any comments from the older commit. I hope I've got it right this time.