Change caret to show language direction

Source/core/editing/Caret.cpp

 /*
  * Copyright (C) 2004, 2008, 2009, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 161 matching lines @@
     RenderObject* caretPainter = caretRenderer(node);
     if (!caretPainter)
         return IntRect();
 
     LayoutRect localRect(rect);
     if (caretPainter->isBox())
         toRenderBox(caretPainter)->flipForWritingMode(localRect);
     return caretPainter->localToAbsoluteQuad(FloatRect(localRect)).enclosingBoundingBox();
 }
 
+void CaretBase::updateDirectionPointer(const RenderObject* caretPainter , LayoutRect& rect)

[Inactive, 2014/05/23 19:57:01]

nit: extra space before comma

[h.joshi, 2014/05/24 02:18:20]

Will remove extra space.

+{
+    // Extra rect are drawn, need to refresh more area.
+    TextDirection containerDirection = toRenderBoxModelObject(caretPainter)->style()->direction();

[Inactive, 2014/05/23 19:57:01]

Would be nicer if this function took  a RenderBoxModelObject then and if the
toRenderBoxModelObject() call were in the callers. This looks unsafe now.

[h.joshi, 2014/05/24 02:18:20]

Okey, will make required changes. I have a question, why its unsafe?

[Inactive, 2014/05/24 02:22:31]

It looks unsafe because the method takes a RenderObject* and you are casting it
to a RenderBoxModelObject* without checking its type. Therefore, it will crash
if the caller passes something else than a RenderBoxModelObject renderer to your
method.

If you let the caller of this method do the casting (if necessary), it will be
more obvious in that context if it is safe to cast to a RenderBoxModelObject* or
not.

+    rect.setWidth(rect.width() + LayoutUnit(2));
+    if (containerDirection  == RTL)
+        rect.setX(rect.x() - LayoutUnit(2));
+}
+
 void CaretBase::repaintCaretForLocalRect(Node* node, const LayoutRect& rect)
 {
     RenderObject* caretPainter = caretRenderer(node);
     if (!caretPainter)
         return;
 
     // FIXME: Need to over-paint 1 pixel to workaround some rounding problems.
     // https://bugs.webkit.org/show_bug.cgi?id=108283
     LayoutRect inflatedRect = rect;
     inflatedRect.inflate(1);
 
+    CaretBase::updateDirectionPointer(caretPainter, inflatedRect);

[Inactive, 2014/05/23 19:57:01]

nit: "CaretBase::" should not be needed.

[h.joshi, 2014/05/24 02:18:20]

Added for clarity, will remove.

     caretPainter->repaintRectangle(inflatedRect);
 }
 
 bool CaretBase::shouldRepaintCaret(const RenderView* view, bool isContentEditable) const
 {
     ASSERT(view);
     bool caretBrowsing = false;
     if (FrameView* frameView = view->frameView()) {
         LocalFrame& frame = frameView->frame(); // The frame where the selection started
         caretBrowsing = frame.settings() && frame.settings()->caretBrowsingEnabled();
@@ skipping 13 matching lines @@
     // it happens after the document has accounted for any editing
     // changes which may have been done.
     // And, we need to leave this layout here so the caret moves right
     // away after clicking.
     m_caretRectNeedsUpdate = true;
 
     if (caretRectChanged)
         return;
 
     if (RenderView* view = node->document().renderView()) {
-        if (shouldRepaintCaret(view, node->isContentEditable(Node::UserSelectAllIsAlwaysNonEditable)))
-            repaintCaretForLocalRect(node, localCaretRectWithoutUpdate());
+        if (shouldRepaintCaret(view, node->isContentEditable(Node::UserSelectAllIsAlwaysNonEditable))) {
+            LayoutRect drawingRect = localCaretRectWithoutUpdate();
+            CaretBase::updateDirectionPointer(caretRenderer(node), drawingRect);

[Inactive, 2014/05/23 19:57:01]

"CaretBase::" should not be needed.

[h.joshi, 2014/05/24 02:18:20]

ditto

+            repaintCaretForLocalRect(node, drawingRect);
+        }
     }
 }
 
 void CaretBase::paintCaret(Node* node, GraphicsContext* context, const LayoutPoint& paintOffset, const LayoutRect& clipRect) const
 {
     if (m_caretVisibility == Hidden)
         return;
 
     LayoutRect drawingRect = localCaretRectWithoutUpdate();
     RenderObject* renderer = caretRenderer(node);
     if (renderer && renderer->isBox())
         toRenderBox(renderer)->flipForWritingMode(drawingRect);
     drawingRect.moveBy(roundedIntPoint(paintOffset));
     LayoutRect caret = intersection(drawingRect, clipRect);
     if (caret.isEmpty())
         return;
 
+    TextDirection containerDirection = toRenderBoxModelObject(renderer)->style()->direction();

[Inactive, 2014/05/23 19:57:01]

It is not clear to me why toRenderBoxModelObject() is safe here but it may be
right, I am not familiar with this code. I would just like to bring this up.
Above it does a isBox() check before casting to RenderBox.

[h.joshi, 2014/05/24 02:18:20]

I am also new to this code so need to check, once all shared comments are fixed,
will start looking into this.

+    LayoutRect directionPointerCaret = caret;
+
+    // Drawing different Caret which shows editing style similar to IE
+    if (containerDirection == LTR)
+        directionPointerCaret.setX(directionPointerCaret.x() + 1);
+    else
+        directionPointerCaret.setX(directionPointerCaret.x() - 2);
+
+    directionPointerCaret.setWidth(LayoutUnit(2));
+    directionPointerCaret.setHeight(LayoutUnit(1));
+
     Color caretColor = Color::black;
 
     Element* element;
     if (node->isElementNode())
         element = toElement(node);
     else
         element = node->parentElement();
 
     if (element && element->renderer())
         caretColor = element->renderer()->resolveColor(CSSPropertyColor);
 
     context->fillRect(caret, caretColor);
+    context->fillRect(directionPointerCaret, caretColor);
+
+    // Update directionPointerCaret points and lets draw one more small rect to match IE.
+    directionPointerCaret.setY(caret.y() + LayoutUnit(1));
+    directionPointerCaret.setWidth(LayoutUnit(1));
+    if (containerDirection == RTL)
+        directionPointerCaret.setX(directionPointerCaret.x() + 1);
+    context->fillRect(directionPointerCaret, caretColor);
 }
 
 void DragCaretController::paintDragCaret(LocalFrame* frame, GraphicsContext* p, const LayoutPoint& paintOffset, const LayoutRect& clipRect) const
 {
     if (m_position.deepEquivalent().deprecatedNode()->document().frame() == frame)
         paintCaret(m_position.deepEquivalent().deprecatedNode(), p, paintOffset, clipRect);
 }
 
 }