Caret missing when an iframe is put into design mode when it already has focus

RESOLVED FIXED in mozilla2.0b8

Status

()

Core
Editor
P2
normal
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Ria Klaassen (not reading all bugmail), Assigned: Ehsan)

Tracking

({regression, testcase})

Trunk
mozilla2.0b8
x86
Windows Vista
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+, blocking1.9.2 -, status1.9.2 wanted)

Details

(URL)

Attachments

(3 attachments, 2 obsolete attachments)

Str:

- go to http://thewestistheverybest.punt.nl/?id=500029&r=1&tbl_archief=&
- scroll down until you see a comment box
- left click in this box

Actual result: caret/cursor missing although I can type
Expected result: caret/cursor present in input field

This works fine with 3.5 but is broken in trunk and Firefox 3.6
Created attachment 425093 [details]
Reduced testcase

Reduced testcase for that page.
Regression range:

works:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090610 Minefield/3.6a1pre
2009-06-10-04-mozilla-central
http://hg.mozilla.org/mozilla-central/rev/90d3e6d2cbb9

broken:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090611 Minefield/3.6a1pre
2009-06-11-04-mozilla-central
http://hg.mozilla.org/mozilla-central/rev/4430cae50dad
Thanks Matt, then this should be the query:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90d3e6d2cbb9&tochange=4430cae50dad
Blocks: 178324
Keywords: regressionwindow-wanted, testcase-wanted
I don't see this bug. Reduced testcases would be nice. This may be duplicate of 534562 or 539323.
Created attachment 425237 [details]
Smaller reduced testcase

Smaller reduced testcase.
Attachment #425093 - Attachment is obsolete: true
Created attachment 425254 [details]
super reduced testcase
Seems like the caret is not being made visible properly when design mode is enabled while the iframe is already focused. A simple hack to make the caret visible at the end of SetDesignMode works ok, but I don't know what the real fix is here.

I suspect bug 534562 may be the same issue.
blocking1.9.2: --- → ?
blocking2.0: --- → ?
status1.9.2: --- → ?
We'd consider a reviewed and baked patch.
blocking1.9.2: ? → -
status1.9.2: ? → wanted
bug 544277 will fix similar problem. but it doesn't care the focus() method of iframe.
Depends on: 544277
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100421 Minefield/3.7a5pre

Problem is still happening; bug 544277 hasn't fixed it.
blocking2.0: ? → final+
Priority: -- → P2
Keywords: testcase

Updated

7 years ago
Blocks: 590077
Assignee: nobody → ehsan
Summary: Caret missing in comment input on http://thewestistheverybest.punt.nl/ → Caret missing when an iframe is put into design mode when it already has focus
Created attachment 486791 [details] [diff] [review]
Patch (v1)

The problem here is that nsHTMLEditor::HasFocus calls nsHTMLEditor::OurWindowHasFocus if the focused content is null, so if that function returns true, the contents of this if block are skipped <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/base/nsEditor.cpp#320>.  Initializing the editor selection (which causes the initialization of the caret) is inside this block.

The fix here is to return the focused content node (or the document element node if OurWindowHasFocus returns true when focused content is null).  I also renamed HasFocus to GetFocusedContent because it makes more sense that way.
Attachment #486791 - Flags: review?(roc)
Whiteboard: [has patch][needs review roc]
Comment on attachment 486791 [details] [diff] [review]
Patch (v1)

+  nsCOMPtr<nsIContent> focusedContent = dont_AddRef(GetFocusedContent());

Why these dont_AddRef calls?
(In reply to comment #12)
> Comment on attachment 486791 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=486791
> Patch (v1)
> 
> +  nsCOMPtr<nsIContent> focusedContent = dont_AddRef(GetFocusedContent());
> 
> Why these dont_AddRef calls?

GetFocusedContent returns already_AddRefed<nsIContent>.  Without dont_AddRef, we'll be leaking one reference when calling GetFocusedContent.
No we won't, because assigning an already_AddRefed to an nsCOMPtr doesn't add a ref.
Created attachment 487250 [details] [diff] [review]
Patch (v2)

Oh, right.  Sorry, my bad.  This patch removes the dont_AddRef calls.
Attachment #486791 - Attachment is obsolete: true
Attachment #487250 - Flags: review?(roc)
Attachment #486791 - Flags: review?(roc)
Attachment #487250 - Flags: review?(roc) → review+
Whiteboard: [has patch][needs review roc] → [needs landing]
http://hg.mozilla.org/mozilla-central/rev/d4f97e748cc4
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
You need to log in before you can comment on or make changes to this bug.