Closed Bug 910729 Opened 11 years ago Closed 10 years ago

Pressing the Apostrophe in a content editable opens up the quick find dialog

Categories

(Toolkit :: Find Toolbar, defect)

x86_64
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla31
Tracking Status
firefox28 --- wontfix
firefox29 --- affected
firefox30 --- affected
firefox31 - verified

People

(Reporter: filip_wieladek, Assigned: evilpie)

References

Details

(Whiteboard: p=0 s=it-31c-30a-29b.2 [qa!])

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/28.0.1500.71 Chrome/28.0.1500.71 Safari/537.36

Steps to reproduce:

Use the following html:

<a  href="http://the.earth.li/~sgtatham/putty/latest/x86/putty.exe">link here </a>
<div contenteditable="true">First Click here and type ' and then click the link download the file and try again</div>

(the test case is http://jsfiddle.net/H4wNh/6/)

1) Press on the link to download the file.
2) Give focus to the content editable
3) Type ' (Apostrophe) 


Actual results:

The quick find opens. It does not open before the link is clicked


Expected results:

The apostrophe should be typed in.
I am changing the importance to major as this is a major usability problem for us. Once a link with a download is clicked, our rich text editor is not useable in firefox. Firefox is the only browser that behaves this way
Severity: normal → major
This bug exists since Firefox3.0
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Version: 26 Branch → Trunk
Component: Untriaged → Find Toolbar
Product: Firefox → Toolkit
Attached patch fixSplinter Review
Was working on this anyway, so I just fixed it.
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
Attachment #806210 - Flags: review?(mdeboer)
Comment on attachment 806210 [details] [diff] [review]
fix

Review of attachment 806210 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/widgets/findbar.xml
@@ +721,5 @@
>                  elt instanceof HTMLEmbedElement)
>                return false;
> +
> +            if (elt.isContentEditable)
> +              return false;

What's the rationale behind this addition? (Or: how on earth did you find this solution? ;) )
Flags: needinfo?(evilpies)
So what this and the surrounding code does is, that it checks if the users is in some kind of input field. Because in that case the user actually wants to type the apostrophe, instead of opening quickfind. My new check basically extends this to an other case where somebody is actually entering text, namely editing a contenteditable element. I however just realized that there might be a problem that I have to look at. I think what could happen is that somebody focuses a node instead a node, which is contenteditable. In this case they still probably want to enter text, but I assume that the "isContentEditable" flag would be false.
Flags: needinfo?(evilpies)
It is important to note that the behaviour is correct initially and breaks only after having pressed on the link. So there might be a state issue somewhere additionally.
Tom, any update on this?
Flags: needinfo?(evilpies)
https://hg.mozilla.org/mozilla-central/rev/32025f35568f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Oh I am sorry, this had the wrong bug number. The right would be bug 917406.
Status: RESOLVED → REOPENED
Flags: needinfo?(evilpies)
Resolution: FIXED → ---
Tom, the right thing to do (usually) is to back out the changeset with the wrong commit message and re-commit it with the corrected message. Mistakes like this will stay in the tree for eternity...
Also, I'm still waiting for your feedback on your original concern (mentioned in comment 5): 
> I think what could happen is that somebody focuses a node instead a node, which is contenteditable. In this case they still probably want to enter text, but I assume that the "isContentEditable" flag would be false.
Flags: needinfo?(evilpies)
Trying to use quote again:

> I think what could happen is that somebody focuses a node instead a node, which is contenteditable. In this case they still probably want to enter text, but I assume that the "isContentEditable" flag would be false.

As it looks horribly wrong in comment 11.
Attachment #806210 - Flags: review?(mdeboer)
Okay sorry, I will try to look at this now.
Flags: needinfo?(evilpies)
Assignee: evilpies → nobody
Status: REOPENED → NEW
THis is a major usability issue in our web application. Is there any hope that this bug is fixed? contenteditable=true is useless for us until the bug is fixed.

Would there be any work around for this issue?
Changing the severity to blocker.

Not only does the quick find toolbar appear, but other editing functionality is gone:

 * Paste (CTRL + V) does not work anymore
 * Cursor is gone
 * Quick find toolbar is opened

Our product is currently unusable with firefox due to this issue. There does also not seem to be any workaround for this issue. Toggling contenteditable from true to false does not fix the issue. 

The only workaround known to us is to refresh the page.
Severity: major → blocker
The product and component are most likely incorrect in this case as it seems to be a general issue with contenteditable=true and not just with the quick find
Target Milestone: mozilla27 → ---
Severity: blocker → normal
@philip.chee could you please file this against the right Product/Component?

This does not only affect the quick find bar but all editing actions (including Copy / Paste)


Could you also explain as to how this severity is only normal? Do you consider losing all modern editing functionality as "normal"?
I think the best thing for you to do is to hop on to IRC:  irc://moznet/developers
and talk directly to Tom Schuster [:evilpie] or Mike de Boer [:mikedeboer]
Has this been happening since 27 and how many users of your application are impacted by this?
Flags: needinfo?(filip_wieladek)
Blocks: fxdesktopbacklog
No longer blocks: fxdesktoptriage
Whiteboard: p=0
@bkerensa, yes, this happens even on the nightly build of firefox. See the description which comes with a test case (http://jsfiddle.net/H4wNh/6/)

We develop a tracking and planning tool and virtually every firefox user is affected by this as we are:

1) using divs with contenteditable=true (for comments + description)
2) providing attachment links which download files on the same page.

Once this issue comes up, the user is unable to paste into the div and the ' is causing the quick find toolbar to appear. The only work around for the user is to refresh the page.

As of today, we have not found any workaround which we could apply code wise to make sure firefox does not enter into the broken state
Flags: needinfo?(filip_wieladek)
Tom, do you have cycles available to look at this?
Flags: needinfo?(evilpies)
Comment on attachment 806210 [details] [diff] [review]
fix

So I just checked "isContentEditable" is true for every child of a contenteditable node. So this fix is correct. I also verified the fix with the last testcase. (Thanks a lot for that!)
Attachment #806210 - Flags: review?(mdeboer)
Flags: needinfo?(evilpies)
@evilpie Are you sure this is the right fix? As I mentioned earlier, when firefox gets into this state, other things become disabled. 

E.g. CTRL + V does not result in a paste anymore. I think the fix should not really go into the toolbar code as it works in the beginning but gets broken after the link is pressed. This should most likely be a firefox common bug, then just the toolbar.

Why is the toolbar not getting this event when everything is fine? E.g. before the link is pressed
Filed Bug 991537
We should land this patch and find somebody else to look at the platform issue. I can only assume that fixing that is going to take longer.
Comment on attachment 806210 [details] [diff] [review]
fix

Review of attachment 806210 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/widgets/findbar.xml
@@ +721,5 @@
>                  elt instanceof HTMLEmbedElement)
>                return false;
> +
> +            if (elt.isContentEditable)
> +              return false;

Perhaps it's OK to just add this check to the if-clause on line 715? That one is also about text-input-type elements, just like this one.
Attachment #806210 - Flags: review?(mdeboer) → review+
Assignee: nobody → evilpies
(In reply to Mike de Boer [:mikedeboer] from comment #26)
> Comment on attachment 806210 [details] [diff] [review]
> fix
> 
> Review of attachment 806210 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/content/widgets/findbar.xml
> @@ +721,5 @@
> >                  elt instanceof HTMLEmbedElement)
> >                return false;
> > +
> > +            if (elt.isContentEditable)
> > +              return false;
> 
> Perhaps it's OK to just add this check to the if-clause on line 715? That
> one is also about text-input-type elements, just like this one.
I don't think that works, we need the element reference.
https://hg.mozilla.org/mozilla-central/rev/201d593499cd
Status: NEW → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Hi Anthony, can you review to determine if further QA verification is required.  Thanks.
No longer blocks: fxdesktopbacklog
Flags: needinfo?(anthony.s.hughes)
Flags: firefox-backlog+
Whiteboard: p=0 → p=0 s=it-31c-30a-29b.2
Tagging this [qa+] to verify this is now fixed and to confirm similar behaviours continue to work. Florin, please assign this to someone on your team for verification.

Tom, does this have automated test coverage? If not, does it need automated tests?
Flags: needinfo?(anthony.s.hughes) → needinfo?(evilpies)
QA Contact: florin.mezei
Whiteboard: p=0 s=it-31c-30a-29b.2 → p=0 s=it-31c-30a-29b.2 [qa+]
QA Contact: florin.mezei → catalin.varga
I verified the bug using the following environment:
FF 31
Build Id: 20140409030203
OS: Win 7 x64, Mac Os X 10.9.2, Ubuntu 13.04 x64

The Quickfind bar no longer opens when typing ' in the "contenteditable" field after pressing the    download link.
The other edit issues( CTLR+V not working after clicking the download link) are still there and Alice logged a different bug 991537 for them.
I also noticed a new issue that is reproducible on older builds, eg typing  ' when the focus is set outside of the contenteditable field is not opening the quickfind bar. I've logged bug 994063 for this issue.
Whiteboard: p=0 s=it-31c-30a-29b.2 [qa+] → p=0 s=it-31c-30a-29b.2
Thanks for looking at this. We sadly don't have tests for this. I am not interesting in writing such a test, because it's 10x more complicated than this fix.
Flags: needinfo?(evilpies)
Whiteboard: p=0 s=it-31c-30a-29b.2 → p=0 s=it-31c-30a-29b.2 [qa!]
(In reply to Tom Schuster [:evilpie] from comment #33)
> Thanks for looking at this. We sadly don't have tests for this. I am not
> interesting in writing such a test, because it's 10x more complicated than
> this fix.

Is this something you want tests for? In other words, if I had someone in QA willing to work on the tests would you be willing to mentor them?
Tom, do you want to handle bug 991537 and bug 994063 in *this* bug, or should we defer that work to a future iteration? In my opinion those issues are likely out of scope for this bug and should be addressed following this iteration. What do you think?
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: