Closed Bug 192566 Opened 22 years ago Closed 21 years ago

selected image in editor shouldn't tint

Categories

(Core :: DOM: Selection, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.4beta

People

(Reporter: Brade, Assigned: mjudge)

References

Details

(Keywords: topembed+, Whiteboard: edt_b3)

Attachments

(1 file, 1 obsolete file)

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.
Keywords: topembedtopembed+
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
to be clear we want to tint the image if it is part of a selection range, just not when it has sprouted handles
> 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.
patch should disable image only selection in editor.
Attachment #118775 - Flags: review+
Attachment #118775 - Flags: superreview?(kin)
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.
Attached patch patch for image frame. 1.2 (deleted) — Splinter Review
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 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+
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....)
i will make sure this is in.
Target Milestone: --- → mozilla1.4beta
Blocks: PhtN5
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
verified fixed in Netscape 7.1 on OSX
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: