[Webkit-unassigned] [Bug 78570] [DRT] Remove all PlainTextController usages in existing tests by adding internal API.

[bugzilla-daemon at webkit.org]

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





--- Comment #26 from Kangil Han <kangil.han at samsung.com>  2012-02-29 00:54:47 PST ---
Okay, I will split up into 3 patches.

First, adding rangeAsText only in this bug.
Second, common cmake changes for WebCoreTestSupport library in a new bug.
Last, efl port changes in a new bug.

(In reply to comment #25)
> (From update of attachment 129399 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=129399&action=review
> 
> i still like to see this patch split up
> 
> I updated the patch at bug 72816 with my current diff
> 
> > Source/WebCore/CMakeLists.txt:96
> > +SET(WebCoreTestSupport_INCLUDE_DIRECTORIES
> > +    "${WebCore_INCLUDE_DIRECTORIES}"
> > +    "${WEBCORE_DIR}/testing"
> > +)
> > +
> 
> I'd prefer to set all WebCoreTestSupport_* variables after the WebCore_* variables (like we do with the features). IMHO this will make it easier to find which files are related to WebCoreTestSupport without searching across the big file.
> 
> > Source/WebCore/CMakeLists.txt:2522
> > +IF (NOT WebCoreTestSupport_LIBRARY_NAME)
> > +    SET(WebCoreTestSupport_LIBRARY_NAME webcoretestsupport)
> > +ENDIF ()
> 
> Can you please add at SET(WebCoreTestSupport_LIBRARY_NAME WebCoreTestSupport) in the root CMakeLists.txt (like the other *_LIBRARY_NAME)
> 
> > Source/WebCore/CMakeLists.txt:2524
> > +WEBKIT_WRAP_SOURCELIST(${WebCore_IDL_FILES} ${WebCore_SOURCES} ${WebCoreTestSupport_IDL_FILES} ${WebCoreTestSupport_SOURCES})
> 
> I'd prefere two call: 1 for WebCore and 1 for WebCoreTestSupport, but its ok for me too, if you prefer it that way :-)
> 
> > Source/WebCore/CMakeLists.txt:2529
> > +ADD_LIBRARY(${WebCoreTestSupport_LIBRARY_NAME} ${WebCore_LIBRARY_TYPE} ${WebCoreTestSupport_SOURCES})
> 
> please create a new LIBRAR_TYPE. WCTS will be usually a static lib only used in DRT
> 
> > Source/WebCore/CMakeLists.txt:2532
> > +ADD_DEPENDENCIES(${WebCoreTestSupport_LIBRARY_NAME} ${JavaScriptCore_LIBRARY_NAME} ${WebCore_LIBRARY_NAME})
> 
> is JSC needed when WebCore is there?
> 
> > Source/WebCore/CMakeLists.txt:2535
> > +TARGET_LINK_LIBRARIES(${WebCoreTestSupport_LIBRARY_NAME} ${WebCore_LIBRARIES})
> 
> Do we need all WebCore libraries for WCTS?

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