Closed Bug 558663 Opened 13 years ago Closed 13 years ago

White squares (handles) for resizing images in wysiwyg editor disappear after the first resizing operation

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- -
status1.9.2 --- wanted
blocking1.9.1 --- -
status1.9.1 --- wanted

People

(Reporter: pietro.brenna, Assigned: tnikkel)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a5pre) Gecko/20100409 Minefield/3.7a5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a5pre) Gecko/20100409 Minefield/3.7a5pre

White squares for resizing images in wysiwyg disappear after the first resizing operation, although if you blur and then refocus they re-appear; this is particularly annoying if you are resizing an image and you want to resize it a bit more.

Reproducible: Always

Steps to Reproduce:
1.Load document in wysiwyg
2.add image and rezize it
Actual Results:  
white squares disappear.

Expected Results:  
white squares should remain.
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100411 Minefield/3.7a5pre

This was working fine with Firefox 3.0 but not with latest 3.5, 3.6 and trunk. Although a regression in Feb 2010 so this is peculiar.
Version: unspecified → Trunk
It seems a regression on 3 Feb 2010. Maybe one of the secret bugs of Timothy Nikkel on that day.
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a7e6a9d1b8ea&tochange=e1e378876084
Status: UNCONFIRMED → NEW
Ever confirmed: true
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Bisected locally, found this is caused by bug 496011, which we took on branches, which explains that part. I will investigate this.
Blocks: 496011
OS: Windows 2000 → All
Hardware: x86 → All
Component: General → Layout
QA Contact: general → layout
So what happens here is that the editor creates a "resizing shadow" to give the user feedback of what the resized image would look like. The resizing shadow content is a child of the html element. When the user releases the mouse the editor sets the resizing shadow class to hidden, which has a display: none style rule. This triggers a recreate for the resizing shadow's frame, and it is the root of an anonymous subtree so bug 496011 kicks in and we recreate the html element. The white resizing rectangles were created in the same way as the resizing shadow, as native anonymous kids, so the regular frame construction path we use to recreate the frames for the html element and down don't ever see the white rectangles.

While digging around in the editor I found that we do not want to recreate the parent when a native anonymous root is removed because the editor does just that in nsHTMLEditor::DeleteRefToAnonymousNode (http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/nsHTMLAnonymousUtils.cpp#232).

So I think what we should do it remove the fix in bug 496011 and instead in RecreateFramesForContent check if the parent frame is a leaf, and if so recreate the parent, because that is what caused the subsequent insert of the anonymous div for the text area to fail in bug 496011. That would fix this bug and keep bug 496011 fixed.
More thoughts on Monday, but my initial reaction is that there are tons of known bugs with the way editor uses anonymous content in ways that fit none of the anonymous content models Gecko is actually designed to deal with; it was basically hacked to work on a Gecko of many years ago and makes assumptions that are just not true now (and weren't then, in many cases).  If we choose to support the editor usage, we need more changes than just backing out bug 496011.  If not, then editor needs to start doing something sane.  My personal preference is the latter.  That raises the obvious question of what "something sane" should be, of course.
Could someone please CC me on bug 496011?
I agree with most of what Boris said in comment 6, but I think we need to fix this bug anyway, and ship the fix to branches as well.
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
Attached patch proposed patchSplinter Review
Fixing the editor badness is something I don't think we can do on the branches, so this is what I'm proposing we do for now.
Assignee: nobody → tnikkel
Attachment #440103 - Flags: review?(bzbarsky)
Blocks: 563665
blocking2.0: ? → final+
Priority: -- → P2
Blocks: 489868
Summary: White squares for resizing images in wysiwyg disappear after the first resizing operation → White squares (handles) for resizing images in wysiwyg editor disappear after the first resizing operation
Blocks: 548795
The patch here also fixes the issue in bug 548795.
With this patch included, the testcase on bug 436454 loops asserting "Resample dirty flag set during sample!" until a crash occurs.

As discussed with bz on irc, I filed bug 572938 on that issue.

I also verified that this issue is trunk only.  I am unable to reproduce the looping or crashing in a branch build that includes the patch on this bug.
Comment on attachment 440103 [details] [diff] [review]
proposed patch

I guess this is ok, as long as all the frames that do native anonymous content are leaves (because otherwise they might end up holding dangling frame pointers)...

Sorry for the terrible lag here; it took me a while to convince myself this is fine.
Attachment #440103 - Flags: review?(bzbarsky) → review+
http://hg.mozilla.org/mozilla-central/rev/d5bc811bad0a
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
We're not going to "block" the branch releases on this, but if you still think we need it (a regression fix seems reasonable) please request branch approval on the patch and we'll get to it.
This bug caused a regression, bug 563665. Should we wait for that to be resolved first?
blocking1.9.1: needed → -
blocking1.9.2: needed → -
Mozilla/5.0 (Windows NT 5.0; rv:2.0b6pre) Gecko/20100913 Firefox/4.0b6pre
Using windows 2000 with the current beta the image disappears when focused and re-appears when blurred. I don't know if this was caused by the patch or by anything else as I have no experience with c++ & mozilla
Please file a new bug for that issue.
Depends on: 727754
(In reply to pietro.brenna from comment #17)
> Mozilla/5.0 (Windows NT 5.0; rv:2.0b6pre) Gecko/20100913 Firefox/4.0b6pre
> Using windows 2000 with the current beta the image disappears when focused
> and re-appears when blurred. I don't know if this was caused by the patch or
> by anything else as I have no experience with c++ & mozilla

If you are still seeing a problem please file a new bug. Thanks.
You need to log in before you can comment on or make changes to this bug.