Merge pull request #1676 from andrew-aladev/character-stack-vs-string…

…-buffer

Replaced character stack with string buffer

Manifest.txt

@@ -298,6 +298,7 @@ test/helper.rb
 test/html/sax/test_parser.rb
 test/html/sax/test_parser_context.rb
 test/html/sax/test_push_parser.rb
+test/html/sax/test_parser_text.rb
 test/html/test_builder.rb
 test/html/test_document.rb
 test/html/test_document_encoding.rb
@@ -325,6 +326,7 @@ test/xml/node/test_subclass.rb
 test/xml/sax/test_parser.rb
 test/xml/sax/test_parser_context.rb
 test/xml/sax/test_push_parser.rb
+test/xml/sax/test_parser_text.rb
 test/xml/test_attr.rb
 test/xml/test_attribute_decl.rb
 test/xml/test_builder.rb

ext/java/nokogiri/internals/NokogiriHandler.java

@@ -17,10 +17,10 @@
  * distribute, sublicense, and/or sell copies of the Software, and to
  * permit persons to whom the Software is furnished to do so, subject to
  * the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be
  * included in all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND,
  * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
  * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
@@ -38,7 +38,6 @@
 import static nokogiri.internals.NokogiriHelpers.stringOrNil;
 
 import java.util.LinkedList;
-import java.util.Stack;
 
 import nokogiri.XmlSyntaxError;
 
@@ -57,12 +56,12 @@
 
 /**
  * A handler for SAX parsing.
- * 
+ *
  * @author sergio
  * @author Yoko Harada <[email protected]>
  */
 public class NokogiriHandler extends DefaultHandler2 implements XmlDeclHandler {
-    Stack<StringBuffer> characterStack;
+    StringBuilder charactersBuilder;
     private final Ruby ruby;
     private final RubyClass attrClass;
     private final IRubyObject object;
@@ -100,7 +99,7 @@ public void setDocumentLocator(Locator locator) {
     @Override
     public void startDocument() throws SAXException {
         call("start_document");
-        characterStack = new Stack();
+        charactersBuilder = new StringBuilder();
     }
 
     @Override
@@ -112,13 +111,7 @@ public void xmlDecl(String version, String encoding, String standalone) {
 
     @Override
     public void endDocument() throws SAXException {
-        StringBuffer sb;
-        if (!characterStack.empty()) {
-            for (int i=0; i<characterStack.size(); i++) {
-                sb = characterStack.get(i);
-                call("characters", ruby.newString(sb.toString()));
-            }
-        }
+        populateCharacters();
         call("end_document");
     }
 
@@ -172,7 +165,7 @@ public void startElement(String uri, String localName, String qName, Attributes
                         args[1] = stringOrNil(ruby, pre);
                         args[2] = stringOrNil(ruby, u);
                     }
-                } 
+                }
                 if (args == null) {
                     args = new IRubyObject[4];
                     args[0] = stringOrNil(ruby, ln);
@@ -187,34 +180,34 @@ public void startElement(String uri, String localName, String qName, Attributes
         }
 
         if (localName == null || localName.equals("")) localName = getLocalPart(qName);
+        populateCharacters();
         call("start_element_namespace",
              stringOrNil(ruby, localName),
              rubyAttr,
              stringOrNil(ruby, getPrefix(qName)),
              stringOrNil(ruby, uri),
              rubyNSAttr);
-        characterStack.push(new StringBuffer());
     }
-    
+
     private static String[] emptyAttrs =
-        {"checked", "compact", "declare", "defer", "disabled", "ismap", "multiple", 
+        {"checked", "compact", "declare", "defer", "disabled", "ismap", "multiple",
          "noresize", "nohref", "noshade", "nowrap", "readonly", "selected"};
-    
+
     private boolean isEmptyAttr(String name) {
         for (String emptyAttr : emptyAttrs) {
             if (emptyAttr.equals(name)) return true;
         }
         return false;
     }
-    
+
     public Integer getLine() {
         return locator.getLineNumber();
     }
-    
+
     public Integer getColumn() {
         return locator.getColumnNumber() - 1;
     }
-    
+
     private boolean isFromFragmentHandler() {
         if (object != null && object instanceof RubyObject) {
             RubyObject rubyObj = (RubyObject)object;
@@ -231,8 +224,7 @@ private boolean isFromFragmentHandler() {
 
     @Override
     public void endElement(String uri, String localName, String qName) throws SAXException {
-        StringBuffer sb = characterStack.pop();
-        call("characters", ruby.newString(sb.toString()));
+        populateCharacters();
         call("end_element_namespace",
              stringOrNil(ruby, localName),
              stringOrNil(ruby, getPrefix(qName)),
@@ -241,24 +233,24 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
 
     @Override
     public void characters(char[] ch, int start, int length) throws SAXException {
-        StringBuffer sb = characterStack.peek();
-        sb.append(ch, start, length);
+        charactersBuilder.append(ch, start, length);
     }
 
     @Override
     public void comment(char[] ch, int start, int length) throws SAXException {
+        populateCharacters();
         call("comment", ruby.newString(new String(ch, start, length)));
     }
 
     @Override
     public void startCDATA() throws SAXException {
-        characterStack.push(new StringBuffer());
+        populateCharacters();
     }
 
     @Override
     public void endCDATA() throws SAXException {
-        StringBuffer sb = characterStack.pop();
-        call("cdata_block", ruby.newString(sb.toString()));
+        call("cdata_block", ruby.newString(charactersBuilder.toString()));
+        charactersBuilder.setLength(0);
     }
 
     @Override
@@ -337,4 +329,10 @@ private IRubyObject document(ThreadContext context) {
         return context.getRuntime().getNil();
     }
 
+    protected void populateCharacters() {
+        if (charactersBuilder.length() > 0) {
+            call("characters", ruby.newString(charactersBuilder.toString()));
+            charactersBuilder.setLength(0);
+        }
+    }
 }

test/helper.rb

@@ -186,6 +186,86 @@ def processing_instruction name, content
           @processing_instructions << [name, content]
         end
       end
+
+      # This document will help us to test the strict order of items.
+
+      class DocWithOrderedItems < XML::SAX::Document
+        attr_reader :items
+
+        def initialize
+          # [
+          #   [ :method_1, argument_1, ... ],
+          #   [ :method_2, argument_2, ... ],
+          #   ...
+          # ]
+          @items = Items.new
+        end
+
+        [
+          :xmldecl,
+          :start_document, :end_document,
+          :start_element, :end_element,
+          :start_element_namespace, :end_element_namespace,
+          :characters, :comment, :cdata_block,
+          :processing_instruction,
+          :error, :warning
+        ]
+        .each do |name|
+          define_method name do |*arguments|
+            @items << [name, *arguments]
+            super *arguments
+          end
+        end
+
+        class Items < Array
+          def get_root_content root_name
+            items          = clone
+            is_inside_root = false
+
+            items.select! do |item|
+              method_name  = item[0]
+              element_name = item[1]
+
+              case method_name
+              when :start_element, :start_element_namespace
+                if element_name == root_name
+                  is_inside_root = true
+                  next false
+                end
+
+              when :end_element, :end_element_namespace
+                is_inside_root = false if element_name == root_name and is_inside_root
+              end
+
+              is_inside_root
+            end
+
+            items
+          end
+
+          def select_methods(names)
+            items = clone
+
+            items.select! do |item|
+              name = item[0]
+              names.include? name
+            end
+
+            items
+          end
+
+          def strip_text! method_names
+            each do |item|
+              method_name = item[0]
+              text        = item[1]
+
+              text.strip! if method_names.include? method_name
+            end
+
+            nil
+          end
+        end
+      end
     end
   end
 end