Opened 12 years ago

Closed 11 years ago

Last modified 10 years ago

#4409 closed defect (fixed)

Safari: dijit.Editor dies while creating an unlabelled Button

Reported by: guest Owned by: alex
Priority: high Milestone: 1.2
Component: Editor Version: 0.9
Keywords: Cc: hannes.wyss@…, bill
Blocked By: Blocking:

Description (last modified by bill)

Hi all! I'm working towards getting dijit.Editor usable in Safari.

In dijit.form.Button:postCreate, the following call to dojo.trim fails:

dojo.trim(this.containerNode.innerText || this.containerNode.textContent); 

The reason for this is that in Safari this.containerNode.innerText returns an empty string. Safari then proceeds with this.containerNode.textContent and passes the result of this second call (undefined) to dojo.trim, where no typechecking is done and the call to undefined.replace fails. Unfortunately this transcends my javascript-foo. What's the canonical way to work around that kind of problem? (it seems that Safari treats an empty String as a false-value - is this a bug in Safari?) However, if I explicitly cast the argument, I get a step further:

dojo.trim(String('' || undefined)) 

Possible patch is attached.

Thanks in advance for your feedback, Hannes

Attachments (1)

dojo.button.patch (949 bytes) - added by guest 12 years ago.

Download all attachments as: .zip

Change History (9)

Changed 12 years ago by guest

Attachment: dojo.button.patch added

comment:1 Changed 12 years ago by Adam Peller

Component: GeneralDijit
Owner: anonymous deleted

probably better style-wise to tack on "or empty string"?

dojo.trim(this.containerNode.innerText || this.containerNode.textContent || '');

though this may not be the right thing to do either. You can probably use that as a workaround while you continue to work on Editor. Why doesn't innerText work on Safari?

comment:2 Changed 12 years ago by Adam Peller

Component: DijitEditor
Owner: set to liucougar

comment:3 in reply to:  2 Changed 12 years ago by guest

Replying to peller:

afics innerText works - it returns an empty string (in my case at least). The problem is that textContent does not.

Meanwhile I've done some research and learned that in Javascript/ECMAScript an empty String is typecast to Boolean False (http://www.ecma-international.org/publications/files/ecma-st/ECMA-262.pdf - 9.2 toBoolean - page 43 (31)).

That leads me to think that the original code does not work as intended, but your suggestion works around that nicely - I'll use that for the time being..

thanks Hannes

comment:4 Changed 12 years ago by bill

Looks like two issues here

  • editor is creating buttons with no labels
  • buttons with no labels crash

The second problem should be fixed as you listed or a similar way, perhaps

dojo.trim(this.containerNode[ "innerText" in this.containerNode ? "innerText" : "textContent" ]

but the first problem should also be fixed, for a11y reasons. But where is the unlabeled button being created? Does this happen with standard editor?

comment:5 Changed 12 years ago by alex

Cc: bill added
Milestone: 1.1
Owner: changed from liucougar to alex
Status: newassigned

I'm gonna fix the trim issue in core and punt back to bill for fixing the dijit side.

comment:6 Changed 12 years ago by Douglas Hays

The code in Button.js has already been fixed.

comment:7 Changed 12 years ago by dylan

Milestone: 1.11.2

mass move of editor issues to 1.2.

comment:8 Changed 11 years ago by bill

Description: modified (diff)
Resolution: fixed
Status: assignedclosed

OK, this seems fixed to me. As Doug said, the Button code is fixed, and probably the buttons don't have labels because they are toolbar buttons and thus just have icons. They do have titles.

Note: See TracTickets for help on using tickets.