Cut function in a readonly textarea

RESOLVED FIXED in Firefox 35



5 years ago
3 years ago


(Reporter: cristianstoica85, Assigned: manu.jain13, Mentored)


31 Branch
Firefox 35
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox35 fixed, fennec35+)


(Whiteboard: [lang=js][good first bug])


(5 attachments)



5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140716183446

Steps to reproduce:

Create a simple html page with an readonly textarea.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "">
<html xmlns="">
        Demo bug for Mozilla Firefox 31 for Android.
    <textarea readonly="readonly" rows="20" cols="20">The quick brown fox jumps over the lazy dog.</textarea>

Actual results:

Open the page with Mozilla and select all text/or any text. Press "cut" button. Text is copied to clipboard and then is removed from the textarea.

Expected results:

"Cut" icon should be disabled/removed from toolbar.


5 years ago
OS: Windows 7 → Android
Hardware: x86_64 → All
tracking-fennec: --- → ?
Ever confirmed: true
Component: General → Text Selection
This should be pretty simple ... in the SelectionHandler routine module, we just need to refine the decision we make as to whether or not a field is editable.
Mentor: markcapella
Whiteboard: [lang=js][good first bug]

Comment 2

5 years ago
Hi, I would like to patch this bug. Please assist to go forward as this is my first bug.Thanks..
Assignee: nobody → manu.jain13
manu: Have you downloaded Firefox source repo and built the desktop version yet? Also, for a Firefox/Android bug such as this, you should have a *nix OS for building, and optimally an Android device for testing.

(And of course this particular patch requires javascript knowledge).

Comment 4

5 years ago
I've built the Firefox desktop version. I have windows and ubuntu OS installed on my PC. I also have a android device. I have some basic javascript knowledge as well. Please guide me forward. Thanks..
Have you built FF and installed / run it on your Android device? Try to reproduce the problem in this bugs description (above) as provided by the reporter ...

You can see for instructions on installing an Android development environment ... and can usually find me (or helpful others) on irc in #introduction and #mobile if you have really basic questions about getting started  :-D

Comment #1 (also above) basically gives away the answer, but I don't want to make it too easy for you ;-)

Comment 6

5 years ago
I've reproduce the problem on my android device. Yes, it also cut icon in readonly textarea.
I've build android development environment on my pc successfully.

Comment 7

5 years ago
I got that function "isElementEditableText"(line 799) of "SelectionHandler routine module" got a bug, that its only checking whether the element is input element or textareaelement.
I add up"[readonly]")==false in "if" condition but it shows none of the icons(i.e copy, select all, etc.) on selecting textarea element.
I didn't understand why all the icon disappears. Please guide. Thanks!!
Without seeing your code, I can only assume that you have a javascript syntax error that's breaking SelectionHandler processing. You can check the android LOGCAT to verify if that's true, you should see error messages there.

Additionally, so I or other reviewers you might work with can help better, you can attach the patch that's not working for us to look at. See:
tracking-fennec: ? → 35+

Comment 9

5 years ago
I have made changes in return statement of "isElementEditableText" function.
I have add one more condition about attribute("readonly") mapping.
But none of the icon appear(i.e copy,select all) on selecting the text in textarea field.
Attachment #8485294 - Flags: feedback+


5 years ago
Attachment #8485294 - Attachment description: SelectionHandler.js → SelectionHandler.js(patch at line 799)
Ok, a couple of things here ... re-read on how to create and attach a patch/diff. (You've just attached the entire source file).

Check on irc in #introduction for me or any other mentors if you need extra help with creating a diff/patch file :-)

Don't flag yourself for feedback+, flag me for review?

Did you examine your android LOGCAT for the cause of your error? You should see something like
JavaScript Error: "aElement.IsAttributeMapped is not a function"

The problem here seems to be that you're trying to use what appears to be a C++ method |.IsAttributeMapped| in a Javascript module. The appropriate Javascript method is |.readOnly|. You can see examples in our code base where we use that here:*.js&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Finally, your code would prevent us from editing in elements that are .readOnly HTMLTextAreaElement's. (That's good).

But we want to block *any* element that is .readOnly. Your code looks like it'll still allow us to edit in .readOnly (text input) HTMLInputElement's.

Comment 11

5 years ago
Posted patch done.patchSplinter Review
Bug is removed successfully.
Attachment #8485383 - Flags: review?(markcapella)
Comment on attachment 8485383 [details] [diff] [review]

Review of attachment 8485383 [details] [diff] [review]:


1) Please remove the comment that says //Bug's patch: Starts from here, and the blank line after it.
2) Please remove the comment that says //Bug's patch: Ends here, and the blank line after it.
3) Please add a valid commit message to the patch. Here's an example from a patch I did last week, (see line #4) :
4) To match mozilla style a little closer, please abbreviate aElement.readOnly==false as simply !aElement.readOnly.

After all that >whew!< do a quick test with this quick page:

I think we'll be done :-)
Attachment #8485383 - Flags: review?(markcapella) → feedback+

Comment 13

5 years ago
I have made all changes that you(Mark Capella) told.
Attachment #8485406 - Flags: review?(markcapella)
Comment on attachment 8485406 [details] [diff] [review]

It looks like this attachment is a diff against your previous diff. Can you just post the basic diff between what's in mozilla-central (the base code) and the final change you're making?

This is close enough that I can fix it up and approve it for you if you're not sure. Just let me know what you'd like to do.
Attachment #8485406 - Flags: review?(markcapella) → feedback+

Comment 15

5 years ago
Ok, I've done that also.
Attachment #8485413 - Flags: review?(markcapella)
Comment on attachment 8485413 [details] [diff] [review]

Review of attachment 8485413 [details] [diff] [review]:

Awesome ! This works and tests out ok for me locally also. The rest is paperwork :)

We need to push your patch to a TRY server to undergo volume / integration testing. Since most new contributors don't have access yet, I've done that for you:

Once that passes we can get the change actually moved into mozilla-central. Basically, your work is done as your patch moves forward now (unless we fail in TRY which I don't expect).

It's time to find another patch to work on :-D

r? to mfinkle ... I've been allowed to r+ things lately, but I'll be cautious for awhile.
Attachment #8485413 - Flags: review?(markcapella)
Attachment #8485413 - Flags: review?(mark.finkle)
Attachment #8485413 - Flags: review+
Comment on attachment 8485413 [details] [diff] [review]

LGTM. Once Try run looks good, you can add the checkin-needed keyword
Attachment #8485413 - Flags: review?(mark.finkle) → review+

Comment 18

5 years ago
Finally, I got my first patch approved. Thanks Mark Capella for your help. Your help really guided me as I'm new to this. Now I'm ready to more bugs and solve them :-)
TRY is good ... let's get it checked in
Keywords: checkin-needed
Keywords: checkin-needed
Whiteboard: [lang=js][good first bug] → [lang=js][good first bug][fixed-in-fx-team]
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=js][good first bug][fixed-in-fx-team] → [lang=js][good first bug]
Target Milestone: --- → Firefox 35
It would be good to add a regression test for this.
Flags: in-testsuite?
A regression test was added in bug 1064657.
Depends on: 1064657
Flags: in-testsuite? → in-testsuite+
status-firefox35: --- → fixed
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.