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.

Sign up for GitHub

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

Jump to bottom

Node#replace fails after duping in jruby #1060

Closed
bryanp opened this issue Mar 2, 2014 · 5 comments
Closed

Node#replace fails after duping in jruby #1060

bryanp opened this issue Mar 2, 2014 · 5 comments
Assignees
@flavorjones
Labels
needs/fix-for-failing-test platform/jruby
Milestone
v1.10.x patch rel...

Comments

@bryanp
Copy link

bryanp commented Mar 2, 2014

Reproducible with code below -- works fine in MRI and Rubinius.

ruby -v
jruby 1.7.9 (1.9.3p392) 2013-12-06 87b108a on Java HotSpot(TM) 64-Bit Server VM 1.6.0_65-b14-462-11M4609 [darwin-x86_64]

require 'nokogiri'

html = "<html><head></head><body><div>replace me</div></body></html>"

doc = Nokogiri::HTML::Document.parse(html).dup

# this fails
doc.css('body').children[0].replace('<div>replaced</div>')

# fixed by doing this
doc = Nokogiri::HTML::Document.parse(html).dup
child = doc.css('body').children[0]
child.document.errors = []
child.replace('<div>replaced</div>')
@knu knu added the jruby label May 7, 2014
flavorjones added a commit that referenced this issue Feb 5, 2015
@flavorjones
Test to reproduce underlying issue for #1060.
ceb4e73
flavorjones added a commit that referenced this issue Feb 5, 2015
@flavorjones
Test reproducing the underlying bug in #1060.
2b42cef 
Signed-off-by: Jake Byman <jakebyman@yahoo.com>
@flavorjones flavorjones self-assigned this Feb 5, 2015
flavorjones added a commit that referenced this issue Feb 7, 2015
@flavorjones
Test reproducing the underlying bug in #1060.
61b303c 
Signed-off-by: Jake Byman <jakebyman@yahoo.com>
@flavorjones flavorjones added this to the v1.10.x patch releases milestone Jan 5, 2019
@flavorjones
Copy link
Member

flavorjones commented Jan 5, 2019 •
edited
Loading

see WIP at branch origin/flavorjones-issue-1060-jruby-document-dup

edit 2019-01-12: branch deleted because it's irrelevant given investigation results below

@flavorjones
Copy link
Member

flavorjones commented Jan 12, 2019 •
edited
Loading

Narrowed this down to this failing test:

  describe "XML::Document#dup" do
    it "should greate a document whose children point to itself" do
      html = "<html><head></head><body><div>replace me</div></body></html>"
      doc = Nokogiri::XML::Document.parse(html)
      dup = doc.dup
      assert_same dup, dup.at_css('div').document

      # should not raise an exception
      dup.at_css('div').parse('<div>replaced</div>')
    end
  end

  describe "HTML::Document#dup" do
    it "should greate a document whose children point to itself" do
      html = "<html><head></head><body><div>replace me</div></body></html>"
      doc = Nokogiri::HTML::Document.parse(html)
      dup = doc.dup
      assert_same dup, dup.at_css('div').document

      # should not raise an exception
      dup.at_css('div').parse('<div>replaced</div>')
    end
  end

@flavorjones
Copy link
Member

@jvshahid I think the root problem here is that the JRuby impl doesn't implement Document#dup to override Node#dup. I'm staring at this code and I don't really know where to start. Any advice?

@flavorjones flavorjones removed this from the v1.10.x patch releases milestone Jan 12, 2019
@jvshahid
Copy link
Member

I just got back from vacation. I will take a look in the next few days.

@flavorjones
Copy link
Member

@jvshahid ❤️

@flavorjones flavorjones mentioned this issue Aug 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@flavorjones flavorjones

Labels
needs/fix-for-failing-test platform/jruby
Projects
None yet
Milestone
v1.10.x patch releases
Development

No branches or pull requests
4 participants
@flavorjones @knu @bryanp @jvshahid