[Webkit-unassigned] [Bug 38603] Expose the editing behavior setting in DRT to test all editing code paths

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=38603


Ojan Vafai <ojan at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56497|review?                     |review-
               Flag|                            |




--- Comment #25 from Ojan Vafai <ojan at chromium.org>  2010-05-19 12:53:37 PST ---
(From update of attachment 56497)
Just a few more nits.

(In reply to comment #24)
> ?(In reply to comment #21)
> > nit: no need for WebCore:: prefix given the using directive at the top of the file.
> 
> It looks like in this case the namespace specifier was necessary because of namespace collision. Would you prefer the original patch or something like "using WebCore::EditingBehavior" at the top

I'm not sure, but I think the prefix is preferred (i.e. original patch).


> > Could you use these enum names in the chromium files as well? Would be better to be consistent and I these names are more clear.
> 
> I'm this case I tried to follow the naming scheme of the Chromium public API. For example (from AssertMatchingEnums.cpp):
> 
> COMPILE_ASSERT_MATCHING_ENUM(WebAccessibilityRoleSliderThumb, SliderThumbRole);
> 
> The API seems to consistently use [EnumName][Value]. I can change it around if you still think I should. :)

Talked to fishd. He agrees with using EditingBehaviorMac and EditingBehaviorWin in the Chromium files. To make it as self-consistent as possible, note that he also suggested abbreviating both Mac and Win. Maybe we could change the JS call to setEditingBehavior to take 'mac' and 'win' as arguments as well? Then, one day, we can make the enum in WebCore consistent as well (a separate patch).


I'm confused by the last two lines here:
+++ b/WebKitTools/DumpRenderTree/wx/LayoutTestControllerWx.cpp
@@ -446,3 +446,8 @@ JSValueRef LayoutTestController::computedStyleIncludingVisitedInfo(JSContextRef,
 void LayoutTestController::authenticateSession(JSStringRef, JSStringRef, JSStringRef)
 {
 }
+
+void LayoutTestController::setEditingBehavior(JSStringRef editingBehavior)
+{
+    // FIXME: Implement
+}
-- 
1.6.3.3

Where did the following come from?
-- 
1.6.3.3

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.