Closed Bug 10673 Opened 21 years ago Closed 21 years ago

[DOGFOOD] Selection refresh problem after apply style.

Categories

(Core :: DOM: Editor, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: buster, Assigned: kinmoz)

References

()

Details

(Whiteboard: [PDT+])

there are editor operations that do not set the selection properly, (or
seem to not set the selection properly, when in fact the selection is
fine) making the editor hard to use.  Those the the cc list, please add specific
examples to this bug.

1) setting/removing text properties with the toolbar buttons.  selection is
always set correctly, but sometimes it disappears.  If you use the key bindings,
you see that selection is in the correct place.

The problem here is nsDocument::SetDisplaySelection should force a repaint if
the selection display boolean is changed.

2) Set text property across a block boundary, an extra character or two gets
selected.  This is bug 10654.
Depends on: 10654
Summary: [DOGFOOD] Actions result in unexpected selections → [DOGFOOD] Selection refresh problem after apply style.
Adjust summary to be more accurate.
Assignee: buster → kin
kin wants this.
To verify that this was just a paint problem, I added the following code
reminicent of the HackForceRedraw stuff.  It works, but it isn't right. If the
root cause of the problem lies elsewhere, this code could be put in place as
a temporary measure.  But that defeats the purpose of incremental painting.
Maybe what should happen is one or more of these:
1) toolbar buttons shouldn't take focus away, or
1a) we should be able to detect when we're losing focus to chrome, and not
disable/enable caret and selection.  See bug 10678.
2) when selection is enabled/disabled, the document should notify someone, and a
repaint of the selected region only should be triggered.
----------------------------------------------------------------------------
nsresult
nsTextEditorFocusListener::Focus(nsIDOMEvent* aEvent)
{
  // turn on selection and caret
  if (mEditor)
  {
    nsCOMPtr<nsIEditor>editor = do_QueryInterface(mEditor);
    if (editor)
    {
      nsCOMPtr<nsIPresShell>ps;
      editor->GetPresShell(getter_AddRefs(ps));
      if (ps)
      {
        ps->SetCaretEnabled(PR_TRUE);

        nsCOMPtr<nsIDOMDocument>domDoc;
        editor->GetDocument(getter_AddRefs(domDoc));
        if (domDoc)
        {
          nsCOMPtr<nsIDocument>doc = do_QueryInterface(domDoc);
          if (doc)
          {
            doc->SetDisplaySelection(PR_TRUE);
          }
        }
// begin hack repaint                <-- begin added code
        nsCOMPtr<nsIViewManager> viewmgr;
        ps->GetViewManager(getter_AddRefs(viewmgr));
        if (viewmgr) {
          nsIView* view;
          viewmgr->GetRootView(view);			// views are not
refCounted
          if (view) {
            viewmgr->UpdateView(view,nsnull,NS_VMREFRESH_IMMEDIATE);
          }
// end hack repaint               <-- end added code
        }
      }
    }
  }
  return NS_OK;
}
Status: NEW → ASSIGNED
Target Milestone: M10
As an experiment, I wrote
a method called nsTextEditorFocusListener::HackForceSelectionRepaint() that
clears the selection and resets it back to what it was before it was cleared,
and added a call to it in nsTextEditorFocusListener::Focus(). This fools the
selection code into repainting itself. Causing the block that contains the
selection to repaint, but not the entire document, so it's a bit better than the
old HackForceRedraw. I've only tested this on windows.


nsresult
nsTextEditorFocusListener::HackForceSelectionRepaint()
{
  nsresult result;
  nsCOMPtr<nsIDOMSelection> selection;
  nsCOMPtr<nsIEditor> editor = do_QueryInterface(mEditor);

  if (!editor)
    return NS_ERROR_NULL_POINTER;

  result = editor->GetSelection(getter_AddRefs(selection));

  if (NS_FAILED(result))
    return result;

  if (!selection)
    return NS_ERROR_NULL_POINTER;

  PRInt32 rcount = 0;

  result = selection->GetRangeCount(&rcount);

  if (NS_FAILED(result))
    return result;

  if (rcount < 1)
    return NS_OK;

  // Clone all the selection ranges!

  nsCOMPtr<nsISupportsArray> arr;

  result = NS_NewISupportsArray(getter_AddRefs(arr));

  if (NS_FAILED(result))
    return result;

  if (!arr)
    return NS_ERROR_NULL_POINTER;

  nsCOMPtr<nsIDOMRange> range;
  nsCOMPtr<nsIDOMRange> newRange;
  nsCOMPtr<nsISupports> isupp;
  PRInt32 i;

  for (i = 0; i < rcount; i++)
  {
    result = selection->GetRangeAt(i, getter_AddRefs(range));

    if (NS_FAILED(result))
      return result;

    if (!range)
      return NS_ERROR_NULL_POINTER;

    result = range->Clone(getter_AddRefs(newRange));

    if (NS_FAILED(result))
      return result;

    if (!newRange)
      return NS_ERROR_NULL_POINTER;

    isupp = do_QueryInterface(newRange);

    if (!isupp)
      return NS_ERROR_NULL_POINTER;

    result = arr->AppendElement(isupp);

    if (NS_FAILED(result))
      return result;
  }

  // Now clear the selection and add the cloned
  // ranges to the selection.

  result = selection->StartBatchChanges();

  if (NS_FAILED(result))
    return result;

  result = selection->ClearSelection();

  if (NS_FAILED(result))
  {
    selection->EndBatchChanges();
    return result;
  }

  for (i = 0; i < rcount; i++)
  {
    isupp = dont_AddRef(arr->ElementAt(i));

    if (!isupp)
    {
      selection->EndBatchChanges();
      return NS_ERROR_NULL_POINTER;
    }

    newRange = do_QueryInterface(isupp);

    if (!newRange)
    {
      selection->EndBatchChanges();
      return NS_ERROR_NULL_POINTER;
    }

    result = selection->AddRange(newRange);

    if (NS_FAILED(result))
      return result;
  }

  result = selection->EndBatchChanges();

  if (NS_FAILED(result))
    return result;

  return NS_OK;
}
We can forget about using the HackForceSelectionRepaint() method above.
Everytime it is called, it has the side effect of clearing the nsTextEditor's
TypeInState.
Looks like buster checked in the view hack repaint code he mentioned. Should I
mark this fixed? Or leave it open till we really fix the problem?
leave it open until a real solution is found.  I still think that when selection
is enabled/disabled, the document should notify someone, and a repaint of the
selected region only should be triggered.
Blocks: 10632
*** Bug 11273 has been marked as a duplicate of this bug. ***
Blocks: 12658
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Added a RepaintSelection() method to the PresShell. The old code, which used to
redraw the entire view in nsTextEditorFocusListener::Focus() and
nsTextEditorFocusListener::Blur(), has been commented out with USE_HACK_REPAINT
and replaced with calls to RepaintSelection().

Fix for bugs #7153, #10673, #12066, #12067, and #12793.

    editor/base/nsEditorEventListeners.cpp    revision 1.89
    editor/base/nsEditorEventListeners.h      revision 1.115
        - Added code to scroll the selection into view
          after processing key events.
        - Commented out the hack that redraws the entire
          view when the focus is gained and lost. Replaced
          the hack code with calls to RepaintSelection().

    layout/base/public/nsIFrameSelection.h    revision 1.115
    layout/base/public/nsIPresShell.h         revision 3.46
    layout/html/base/src/nsPresShell.cpp      revision 3.167
        - Added ScrollSelectionIntoView() and RepaintSelection() methods.

    layout/base/src/nsRangeList.cpp    revision 1.123
        - Added implementation for ScrollSelectionIntoView() and
          RepaintSelection().
        - Check for NULL primary frame in GetFocusNodeRect().
Status: RESOLVED → VERIFIED
verified in 9/2 build.
Whiteboard: [PDT+]
Putting on [PDT]+ radar.
You need to log in before you can comment on or make changes to this bug.