Closed
Bug 558663
Opened 15 years ago
Closed 14 years ago
White squares (handles) for resizing images in wysiwyg editor disappear after the first resizing operation
Categories
(Core :: Layout, defect, P2)
Core
Layout
Tracking
()
People
(Reporter: pietro.brenna, Assigned: tnikkel)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: regression, testcase)
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•15 years ago
|
||
Comment 2•15 years ago
|
||
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
Comment 3•15 years ago
|
||
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
Updated•15 years ago
|
Keywords: regression,
testcase
Updated•15 years ago
|
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Assignee | ||
Comment 4•15 years ago
|
||
Bisected locally, found this is caused by bug 496011, which we took on branches, which explains that part. I will investigate this.
Assignee | ||
Updated•15 years ago
|
Component: General → Layout
QA Contact: general → layout
Assignee | ||
Comment 5•15 years ago
|
||
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.
Comment 6•15 years ago
|
||
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.
Comment 7•15 years ago
|
||
Could someone please CC me on bug 496011?
Comment 8•15 years ago
|
||
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.
Updated•15 years ago
|
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
Assignee | ||
Comment 9•15 years ago
|
||
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)
Updated•15 years ago
|
status1.9.1:
--- → wanted
status1.9.2:
--- → wanted
blocking2.0: ? → final+
Priority: -- → P2
Assignee | ||
Updated•14 years ago
|
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
Comment 11•14 years ago
|
||
The patch here also fixes the issue in bug 548795.
Comment 12•14 years ago
|
||
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 13•14 years ago
|
||
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+
Assignee | ||
Comment 14•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment 15•14 years ago
|
||
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.
Assignee | ||
Comment 16•14 years ago
|
||
This bug caused a regression, bug 563665. Should we wait for that to be resolved first?
Reporter | ||
Comment 17•14 years ago
|
||
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
Assignee | ||
Comment 18•14 years ago
|
||
Please file a new bug for that issue.
Assignee | ||
Comment 19•13 years ago
|
||
(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.
Description
•