i18n_viz icon indicating copy to clipboard operation
i18n_viz copied to clipboard

Fix nested tags, make tests pass

Open atodorov opened this issue 7 years ago • 2 comments

@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:

    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.

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.

atodorov avatar Jun 01 '17 21:06 atodorov

@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 ?

atodorov avatar Jun 05 '17 11:06 atodorov

ok, I think I got it working now. Apparently we now need to install the thing called geckodriver.

atodorov avatar Jun 05 '17 19:06 atodorov