Opened 11 years ago
Closed 11 years ago
#10653 closed defect (fixed)
[regression] InlineEditBox: error with special chars
Reported by: | joshilay | Owned by: | Douglas Hays |
---|---|---|---|
Priority: | high | Milestone: | 1.5 |
Component: | Dijit | Version: | 1.4.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
With dojo 1.4, InlineEditBox with an initial value containing special characters like < > & is not being displayed correctly. Instead page is showing < etc.
Attachments (2)
Change History (18)
comment:1 follow-up: 2 Changed 11 years ago by
Milestone: | 1.4.1 → tbd |
---|---|
Priority: | high → normal |
severity: | major → normal |
comment:2 Changed 11 years ago by
Replying to peller:
You'll need to give a little more detail, preferably with an example. Which code is not processing entities like < ?
I added a test case which shows with inline edit box embedded fonts are not working but outside it embedded fonts are working . I am using FF 3.5.7
comment:3 Changed 11 years ago by
Component: | General → Dijit |
---|---|
Milestone: | tbd → 1.4.1 |
Owner: | changed from anonymous to Douglas Hays |
Summary: | Error with dojo 1.4 embedded fonts like < → [regression] InlineEditBox: error with special chars |
Thanks for attaching the test case. One key thing that you left out of your original report is that you were talking about dijit, and specifically about InlineEditBox. :-)
This worked in 1.3 and is broken in 1.4, starting with [20659]. Doug, can you look at this? Might be worth putting into 1.4.1.
comment:4 Changed 11 years ago by
Resolution: | → invalid |
---|---|
Status: | new → closed |
In 1.3, the renderAsHtml flag defaulted to false but was being ignored. If you want the actual HTML entities to show on the page, then set renderAsHtml="true".
comment:5 Changed 11 years ago by
Milestone: | 1.4.1 → tbd |
---|
comment:6 Changed 11 years ago by
Resolution: | invalid |
---|---|
Status: | closed → reopened |
comment:7 Changed 11 years ago by
By using renderAsHtml="true", inlineeditbox shows special characters like < correctly but when user clicks on the inlineeditbox to edit text, it again shows "<" instead of correct value.
comment:8 Changed 11 years ago by
Resolution: | → invalid |
---|---|
Status: | reopened → closed |
It is behaving correctly. The user should be able to enter:
this is <b><bold></b>
and the HTML will be rendered as
this is <bold>
comment:9 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Resolution: | invalid |
Status: | closed → reopened |
This seems broken to me too.
renderAsHtml=true is inappropriate here, because that setting is for when the editor (ie: the widget shown when you click the InlineEditBox) handles HTML, which in practice means dijit.Editor. Since this test case is using the default dijit.TextBox editor, it should stick to the default renderAsHtml=false.
Saving in renderAsHtml=false mode is working correctly: the user can enter <foo> into the TextBox and when editing is finished it still shows up on the screen as <foo>.
However, the problem though is to specify an initial value of the InlineEditBox that contains special characters. How does a developer specify the initial value as <Inside Inline Edit Box>? I don't see any way. In any case, markup like this doesn't work in 1.4:
<span dojoType="dijit.InlineEditBox"> < Inside Inline Edit Box> </span>
That it worked in 1.3 and I think it makes sense.
Changed 11 years ago by
Attachment: | 10653.patch added |
---|
add initial value filtering if renderAsHtml=false, and created additional automated tests
comment:10 Changed 11 years ago by
Milestone: | tbd → 1.5 |
---|
I attached a patch file for trunk. Needs verification.
comment:11 Changed 11 years ago by
Looks like it will work, although it seems strange to have a this.attr('value', ...)
call at creation, as the main thing it does is just setting innerHTML to itself. (Note that if the caller explicitly sets the value parameter, _setValueAttr() will be called by the widget infrastructure, so in that case too calling this.attr() from postMixInProperties() is superflous.) I think you could replace the code in postMixInProperties() with:
if(!this.value){ this.value = dojo.trim(this.renderAsHtml ? this.displayNode.innerHTML : (this.displayNode.innerText||this.displayNode.textContent)); } if(!this.value){ this.displayNode.innerHTML = this.noValueIndicator; }
I thought about reducing it further by having a value getter method (since there's no real reason to extract the value from innerHTML on creation), although it's complicated by that noValueIndicator:
_getValueAttr: function(){ return this.displayNode.innerHTML =- this.noValueIndicator ? "" : ( this.renderAsHtml ? this.displayNode.innerHTML : (this.displayNode.innerText||this.displayNode.textContent) ); }
(Would need to change the edit() method to call attr("value") instead of accessing this.value directly.)
I'm worried about that breaking because the [this.displayNode.innerHTML == this.noValueIndicator
test might return false just because of spurious differences like case, attribute order, <img> vs <img/> etc.
comment:12 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:13 Changed 11 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
This still seems broken to me, and actually robot/InlineEditBox.html is failing (in both [21288] and the latest on trunk), on at least FF and Chrome.
Regardless of the renderAsHTML setting, the initial DOM in the <div dojoType=dijit.InlineEditBox> node should not be modified by InlineEditBox widget until the user clicks to edit and then saves.
The test file is:
<h2>Complex renderAsHtml="false"</h2> <div id="renderAsHtml_false" dojoType="dijit.InlineEditBox" renderAsHtml="false" width="400px" style="width:400px;"> <B>not bold</B><B>not bold</B>&lt;input&gt; </div> <hr style="width:100%;"> <h2>Complex renderAsHtml="true"</h2> <div id="renderAsHtml_true" dojoType="dijit.InlineEditBox" renderAsHtml="true" width="400px" style="width:400px;"> <B>not bold</B><B>bold</B>&lt;input&gt; </div>
That means that when the page is rendered it should appear as:
Complex renderAsHtml="false
<B>not bold</B>not bold&<input&>
Complex renderAsHtml="true"
<B>not bold</B>not bold&<input&>
In other words, on initial page load it should display the same regardless of whether the markup is <div dojoType=dijit.InlineEditBox> or just <div>.
comment:14 Changed 11 years ago by
I disagree. The renderAsHtml=false test with the initial bold text was intentionally added to make sure the widget was able to modify user text to correct an invalid situation. With renderAsHtml=false, the non-edit value of <B> is translated to <B> when editing since that's how's its rendered and thus the user is just changing text and not HTML. If a non-edit value of <B> is allowed, then it would not be discernable from the former <B>. The only way for this to work would be for the non-edit value of <B> to not be translated to <B> when editing, which would make editing confusing since the displayed characters do not match the editing characters which is a design-point for renderAsHtml=false.
comment:15 Changed 11 years ago by
comment:16 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I see... so I mispoke above, what I should have said was that:
<div dojoType="dijit.InlineEditBox" renderAsHtml="false" ...> <b>bold</b> </div>
should be unsupported (leading to undefined behavior). I understand that you want to correct for developer errors like malformed HTML:
<div dojoType="dijit.InlineEditBox" renderAsHtml="false" ...> sticks & stones </div>
which should have been:
<div dojoType="dijit.InlineEditBox" renderAsHtml="false" ...> sticks & stones </div>
However, I just don't want to add that overhead to the InlineEditBox code, in particular the performance hit of modifying the InlineEditBox.innerHTML on widget instantiation. It's possible the error above is a common error but I don't think it is, as it's no different than others times that "plain text" (with possible reserved characters) is inserted into a web page.
So, I'll mark this ticket as fixed since the reported issue (the last example above) is now working correctly, but I'm going to roll back the part about correcting malformed markup.
You'll need to give a little more detail, preferably with an example. Which code is not processing entities like < ?