Change caret to show language direction

Source/core/editing/Caret.cpp

Index: Source/core/editing/Caret.cpp
diff --git a/Source/core/editing/Caret.cpp b/Source/core/editing/Caret.cpp
index 2b3c1d6ccf2ebb25d5d785e3293dcadc09b02066..144363d8012ac8ed86574da5aef5aac2ef6433bf 100644
--- a/Source/core/editing/Caret.cpp
+++ b/Source/core/editing/Caret.cpp
@@ -179,6 +179,15 @@ IntRect CaretBase::absoluteBoundsForLocalRect(Node* node, const LayoutRect& rect
     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);
@@ -190,6 +199,7 @@ void CaretBase::repaintCaretForLocalRect(Node* node, const LayoutRect& rect)
     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);
 }
 
@@ -223,8 +233,11 @@ void CaretBase::invalidateCaretRect(Node* node, bool 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);
+        }
     }
 }
 
@@ -242,6 +255,18 @@ void CaretBase::paintCaret(Node* node, GraphicsContext* context, const LayoutPoi
     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;
@@ -254,6 +279,14 @@ void CaretBase::paintCaret(Node* node, GraphicsContext* context, const LayoutPoi
         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