Implement <select> parser “relaxation” — for “customizable” <select> #113
Conversation
sideshowbarker
force-pushed
the
sideshowbarker/relaxed-select-parsing
branch
from
7a83606 to
cdf2934
Compare
This change make our test harness skip 13 tests for foreign-content fragment parsing that have been failing from some time now. They’re not regressions — or not recent regressions at least — so, we can essentially treat them as “known to fail” for the time being.
This change, in order to support the “customizable” <select> element: - “relaxes” parsing rules in <select> (to allow more elements as children) - adds the <selectedcontent> element for cloning selected <option> content ElementName.java ---------------- - Add <selectedcontent> element with TreeBuilder.SELECTEDCONTENT group TreeBuilder.java ---------------- - Add SELECTEDCONTENT group constant and IN_SELECT_IN_CONTENT mode - Track selectedContentPointer and activeOptionStackPos for cloning - Allow <div>, <span>, and other elements in <select> in “relaxed” mode - Deep clone <option> content to <selectedcontent> when <option> closes - Handle <selectedcontent> special-casing (store pointer, no stack push) SAXTreeBuilder.java ------------------- - Implement clearSelectedContentChildren() for clearing before clone - Implement deepCloneChildren() and deepCloneNode() for <option> cloning ParentNode.java --------------- - Add clearChildren() method, to support clearing <selectedcontent> html5lib-tests submodule ------------------------ - Updated to pull in corresponding tests for “relaxed” <select> parsing
sideshowbarker
force-pushed
the
sideshowbarker/relaxed-select-parsing
branch
from
cdf2934 to
a1d8a3d
Compare
|
Sorry I missed this over the holidays. |
No worries — there's no real rush on it |
hsivonen
left a comment
hsivonen
left a comment
There was a problem hiding this comment.
Sorry about the delay.
As discussed on Matrix, I think the way this patch makes the cloning work on what the parser saw is incorrect and the spec instead requires the cloning to work on what's in the live DOM, which may be different if the option element opens, the network stalls, setTimeout modifies the DOM, and network resumes.
Further remarks in an inline comment.
| break; | ||
| case CHARACTERS: | ||
| // Clone the characters | ||
| String text = node.toString(); |
There was a problem hiding this comment.
It seems inefficient to create an intermediate String object compared to coping from a char array to a char array directly.
| } | ||
|
|
||
| @Override | ||
| protected void clearSelectedContentChildren() throws SAXException { |
There was a problem hiding this comment.
Will there be similar changes to the other subclasses of TreeBuilder?
|
|
||
| // Stack to track the current parent in selectedcontent for element cloning | ||
| // When we're inside an active option, we push cloned elements here | ||
| private ArrayList<T> selectedContentCloneStack = new ArrayList<T>(); |
There was a problem hiding this comment.
Per spec, it seems that the cloning step runs on what's in the DOM and not on what the parser saw.
So I think the tree builder shouldn't keep track of selectedcontent but should have a hook for popping option to call a method that performs the cloning.
In Gecko, the method would queue a tree op to perform the cloning on the main-thread DOM.
The streaming SAX mode would ignore the cloning.
The other Java tree builders should override the method to perform the cloning on their respective tree models.
| // case TreeBuilder.FOREIGNOBJECT_OR_DESC: | ||
| // return "FOREIGNOBJECT_OR_DESC"; | ||
| // case TreeBuilder.SELECTEDCONTENT: | ||
| // return "SELECTEDCONTENT"; |
There was a problem hiding this comment.
It seems to me that this isn't needed when cloning moves away from the parsing algorithm internals.
This change, in order to support the “customizable”
selectelement:select(to allow more elements as children)selectedcontentelement for cloning selectedoptioncontentElementName.javaselectedcontentelement withTreeBuilder.SELECTEDCONTENTgroupTreeBuilder.javaSELECTEDCONTENTgroup constant andIN_SELECT_IN_CONTENTmodeselectedContentPointerandactiveOptionStackPosfor cloningdiv,span, and other elements inselectin “relaxed” modeoptioncontent toselectedcontentwhenoptionclosesselectedcontentspecial-casing (store pointer, no stack push)SAXTreeBuilder.javaclearSelectedContentChildren()for clearing before clonedeepCloneChildren()anddeepCloneNode()foroptioncloningParentNode.javaclearChildren()method, to support clearingselectedcontenthtml5lib-tests(submodule)selectparsing