Closed Bug 192566 Opened 21 years ago Closed 20 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.
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: 20 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: