Closed
Bug 192566
Opened 22 years ago
Closed 21 years ago
selected image in editor shouldn't tint
Categories
(Core :: DOM: Selection, defect)
Core
DOM: Selection
Tracking
()
VERIFIED
FIXED
mozilla1.4beta
People
(Reporter: Brade, Assigned: mjudge)
References
Details
(Keywords: topembed+, Whiteboard: edt_b3)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
danm.moz
:
review+
sfraser_bugs
:
superreview+
|
Details | Diff | Splinter Review |
When an image is selected in the editor, we get the resize handles. It'd be
nice to not have the tinting when we have the resize handles.
Updated•22 years ago
|
Updated•22 years ago
|
Whiteboard: edt_b3
well the easy way is to check iseditor before drawing the selection. the
"right" way is to add those hooks we talked about to allow embedders to turn off
selection of any given item.
Status: NEW → ASSIGNED
Comment 2•22 years ago
|
||
to be clear we want to tint the image if it is part of a selection range, just
not when it has sprouted handles
Comment 3•22 years ago
|
||
> to be clear we want to tint the image if it is part of a selection range, just
> not when it has sprouted handles
Said differently, we don't want to tint if the image is alone in the selection.
Attachment #118775 -
Flags: review+
Reporter | ||
Updated•22 years ago
|
Attachment #118775 -
Flags: superreview?(kin)
Comment 5•22 years ago
|
||
Comment on attachment 118775 [details] [diff] [review]
patch for nsImageFrame.cpp in layout\html\base\src
>Index: nsImageFrame.cpp
>===================================================================
>+#if IMAGE_EDITOR_CHECK
>+ //check to see if this frame is in an editor context
>+ //isEditor check. this needs to be changed to have better way to check
Please mention the bug number in the comment
>+ if (displaySelection == nsISelectionDisplay::DISPLAY_ALL)
>+ {
>+ SelectionDetails *details = 0;
>+ nsCOMPtr<nsIFrameSelection> frameSelection;
These lines are unused.
>+ nsCOMPtr<nsISelectionController> selCon;
>+ result = GetSelectionController(aPresContext, getter_AddRefs(selCon));
>+ if (NS_SUCCEEDED(result) && selCon)
>+ {
>+ nsCOMPtr<nsISelection> selection;
>+ result = selCon->GetSelection(nsISelectionController::SELECTION_NORMAL, getter_AddRefs(selection));
>+ if (NS_SUCCEEDED(result) && selection)
>+ {
>+ PRInt32 rangeCount;
>+ selection->GetRangeCount(&rangeCount);
>+ if (rangeCount == 1) //if not one then let code drop to nsFrame::Paint
>+ {
>+ nsCOMPtr<nsIContent> parentContent;
>+ mContent->GetParent(*getter_AddRefs(parentContent));
>+ if (parentContent)
>+ {
>+ PRInt32 thisOffset;
>+ parentContent->IndexOf(mContent, thisOffset);
>+ nsCOMPtr<nsIDOMNode> parentNode = do_QueryInterface(parentContent);
>+ nsCOMPtr<nsIDOMNode> rangeNode;
>+ PRInt32 rangeOffset;
>+ selection->GetAnchorNode(getter_AddRefs(rangeNode));//anchor because there is one range there
>+ selection->GetAnchorOffset(&rangeOffset);
>+ if (parentNode && rangeNode && (rangeNode == parentNode) && rangeOffset == thisOffset)
>+ {
>+ selection->GetFocusNode(getter_AddRefs(rangeNode));//anchor because there is one range there
>+ selection->GetFocusOffset(&rangeOffset);
>+ if ((rangeNode == parentNode) && (rangeOffset == (thisOffset +1))) //+1 since that would mean this whole content is selected only
>+ return NS_OK; //do not allow nsFrame do draw any further selection
>+ }
>+ }
>+ }
>+ }
>+ }
>+ }
>+#endif
> return nsFrame::Paint(aPresContext, aRenderingContext, aDirtyRect, aWhichLayer, nsISelectionDisplay::DISPLAY_IMAGES);
We should file a bug to get a better way to have selection communicate with
stuff like the resize handles.
Attachment #118775 -
Flags: superreview?(kin) → superreview+
Comment on attachment 118775 [details] [diff] [review]
patch for nsImageFrame.cpp in layout\html\base\src
So this patch retrieves the endpoints of the range using anchor and focus
methods and assumes that the anchor is *before* the image. It should retrieve
the range and get the endpoints from the range if it wants a guaranteed
ordering ... that is, it's entirely possible for the anchor point to be after
the image, in which case the hiliting could still happen even if the only thing
selected is the image.
Also, shouldn't we only display handles if we click directly on the image?
abc <img> def
It just seems a bit odd to me that when you place the caret to the left of the
image and hold down the shift key and press right arrow, that it displays the
handles, and then a 2nd right arrow key causes the hiliting to change to the
tinted rendering. The same thing can also happen when drag hiliting from before
the image and ending in the text after it.
goes straight to the range rather than asking for focus and anchor. as kin
pointed out it IS possible to select just the image while selecting backwards.
Attachment #118775 -
Attachment is obsolete: true
Comment 8•22 years ago
|
||
Comment on attachment 121339 [details] [diff] [review]
patch for image frame. 1.2
sr=sfraser. Please file another bug to track the showing of the grippies vs.
the selection display.
Attachment #121339 -
Flags: superreview+
Attachment #121339 -
Flags: review+
Comment 9•22 years ago
|
||
Um.. is there a bug on adding hooks or whatever instead of those ifdefs?
(apart from the fact that we _really_ need a way to check the "isEditor" thing
without relying on a random undocumented property of the selection type flag....)
Assignee | ||
Comment 10•22 years ago
|
||
i will make sure this is in.
Target Milestone: --- → mozilla1.4beta
Comment 11•21 years ago
|
||
The attached patch was checked in on 2003-04-22 16:18 and it works. Looks like
mjudge forgot to mark this bug fixed back then.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•