Closed
Bug 296677
Opened 20 years ago
Closed 20 years ago
nsParser::ParseFragment screws up setting <textarea>'s innerHTML
Categories
(Core :: DOM: HTML Parser, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: mrbkap, Assigned: mrbkap)
References
Details
Attachments
(1 file, 2 obsolete files)
|
12.58 KB,
patch
|
jst
:
review+
peterv
:
superreview+
asa
:
approval1.8b3+
|
Details | Diff | Splinter Review |
As noted in bug 293162 comment 1, <BODY> gets added to the textarea when we
remove the hack that makes us treat the incoming innerHTML. This is because for
bug 283036, we append a <BODY> string to the context to flush tags out of CNavDTD.
|
Assignee | |
Comment 1•20 years ago
|
||
This goes directly through the DTD instead of trying to add it at the tokenizer
level, which doesn't work for <textarea>. I almost think a FlushMisplaced()
function on the DTD would be cleaner, but this works too.
|
|
Assignee | |
Updated•20 years ago
|
Status: NEW → ASSIGNED
|
|
Assignee | |
Comment 2•20 years ago
|
||
Comment on attachment 185374 [details] [diff] [review]
patch v1
Johnny, what do you think?
Attachment #185374 -
Flags: review?(jst)
|
|
Assignee | |
Comment 3•20 years ago
|
||
I'm morphing this bug into the bug to make nsParser::ParseFragment() not screw
up on <textarea>. Sorry Johnny :-)
Summary: <BODY> gets added to fragments involving <textarea> → nsParser::ParseFragment screws up setting <textarea>'s innerHTML
|
|
Assignee | |
Comment 4•20 years ago
|
||
This fixes things so that when bug 265367 is fixed, we'll be able to simply
remove nsHTMLTextAreaElement::GetInnerHTML and
nsHTMLTextAreaElement::SetInnerHTML.
This does everything patch v1 does, and adds a IgnoreTags() function to the
HTML fragment content sink interface that tells the interface to ignore N tags
from the content. Then, when we parse the context, we make sure that we got all
the way to the end of the context string, and if we didn't, we make a gigantic
leap of faith, and assume that there's exactly one tag left of the context
stack (note: this invarient actually does hold in the tokenizer, given a well
formed context string, which we'll have).
Attachment #185374 -
Attachment is obsolete: true
Attachment #185514 -
Flags: review?(jst)
|
|
Assignee | |
Updated•20 years ago
|
Attachment #185374 -
Flags: review?(jst)
|
||
Comment 5•20 years ago
|
||
Comment on attachment 185514 [details] [diff] [review]
patch v2
+
+ /**
+ * This method is used to tell the content sink that there are tags that are
+ * going to be in the content that should be ignored. This is mainly for
cases
+ * such as <textarea> in HTML, where the actual <textarea> tag gets added
+ * after the call to WillBuildContent().
+ *
+ * @param aDepth The number (depth) of tags that should be ignored.
+ */
+ NS_IMETHOD IgnoreTags(PRInt32 aDepth) = 0;
Maybe name this SetIgnoreTagsDepth()?
- In nsHTMLFragmentContentSink::OpenContainer():
+ if (mIgnoreDepth > 0) {
+ --mIgnoreDepth;
+ return result;
+ }
That seems reasonable, but don't you want to keep track of the current depth
too and make sure things are balanced (i.e. what should happen if we get a
ignore depth of 1 and we pares something like "<a><b>foo</b></a><c>...</c>"
should both a and c be ignored, or just a? I'd say this is a case we don't want
to support but we should have the code to throw the appropriate error if we do
see a case like this, no?
The rest looks fine to me.
|
|
Assignee | |
Comment 6•20 years ago
|
||
(In reply to comment #5)
> That seems reasonable, but don't you want to keep track of the current depth
> too and make sure things are balanced (i.e. what should happen if we get a
> ignore depth of 1 and we pares something like "<a><b>foo</b></a><c>...</c>"
> should both a and c be ignored, or just a? I'd say this is a case we don't
jst and I discussed this. The function is going to be renamed to
IgnoreFirstContainer() and commented to be the hack that it is.
|
|
Assignee | |
Comment 7•20 years ago
|
||
As promised, this doesn't even pretend to add a generally useful function to
the fragment content sink.
Attachment #185514 -
Attachment is obsolete: true
Attachment #185692 -
Flags: review?(jst)
|
|
Assignee | |
Updated•20 years ago
|
Attachment #185514 -
Flags: review?(jst)
|
|
||
Comment 8•20 years ago
|
||
Comment on attachment 185692 [details] [diff] [review]
patch v3
r=jst
Attachment #185692 -
Flags: review?(jst) → review+
|
|
Assignee | |
Updated•20 years ago
|
Attachment #185692 -
Flags: superreview?(peterv)
|
||
Updated•20 years ago
|
Attachment #185692 -
Flags: superreview?(peterv) → superreview+
|
|
Assignee | |
Comment 9•20 years ago
|
||
Comment on attachment 185692 [details] [diff] [review]
patch v3
This should probably go in as soon as possible. These are some of the last
changes required to make textarea's innerHTML be happy. They're about
medium-risk because of the dependence on lack of bugs in the tokenizer.
Attachment #185692 -
Flags: approval1.8b3?
|
||
Updated•20 years ago
|
Attachment #185692 -
Flags: approval1.8b3? → approval1.8b3+
|
|
Assignee | |
Comment 10•20 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Could this have caused the ~10ms Tp hit in btek?
You need to log in
before you can comment on or make changes to this bug.
Description
•