Added in http://trac.webkit.org/changeset/57986 and used in the following tests: fast/lists/ol-nested-items-dynamic-insert.html fast/lists/ol-nested-items-dynamic-remove.html fast/lists/ol-nested-items.html fast/lists/ol-nested-list-dynamic-insert.html fast/lists/ol-nested-list-dynamic-remove.html fast/lists/ol-nested-list.html
Created attachment 61605 [details] Patch I thought to split up the Mac and Windows patches and use Bug #42054 as a meta bug. With insight from Adam Roben, I updated the patch based on Darin Adler's comment in Bug #42054 <https://bugs.webkit.org/show_bug.cgi?id=42054#c4>. I thought to keep the implementation of both ports similar for consistency. So, I added a similar DOMElement conversion function to the Windows port, WebFrame::elementFromJS (I am open to naming suggestions). I was not sure where to place WebFrame::elementFromJS. My original thought was to add a class method to DOMElement (similar to the class method added to the WebDOMElementOperationsPrivate category of DOMElement in the Mac patch - see bug #37929), but I felt that keeping the function in some kind of private interface better demarcated that this function is not for public usage. I am open to suggestions on this patch.
Attachment 61605 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/3576007
Make sure you touch WebKit.idl as part of your patch so the EWS bot rebuilds the idl files you need.
Comment on attachment 61605 [details] Patch > +HRESULT WebFrame::elementFromJS(JSContextRef context, JSValueRef nodeObject, IDOMElement **element) > +{ > + if (!element) > + return E_POINTER; > + > + *element = 0; > + > + if (!context) > + return E_FAIL; > + > + JSLock lock(SilenceAssertionsOnly); > + ExecState* execState = toJS(context); > + if (!nodeObject) > + return E_FAIL; I'd null-check nodeObject at the same time as context. > + JSValue jsValue = toJS(execState, nodeObject); I don't think the execState or jsValue local variables are useful. > @@ -114,4 +114,6 @@ interface IWebFramePrivate : IUnknown > HRESULT paintScrollViewRectToContextAtPoint([in] RECT rect, [in] POINT pt, [in] OLE_HANDLE deviceContext); > > HRESULT renderTreeAsExternalRepresentation([in] BOOL forPrinting, [out, retval] BSTR* result); > + > + [local] HRESULT elementFromJS([in] JSContextRef context, [in] JSValueRef nodeObject, [out, retval] IDOMElement** element); Putting this on IWebViewPrivate would be a little more conventional, I think. > + if (!framePrivate) > + return JSRetainPtr<JSStringRef>(); You can say "return 0" instead of "return JSRetainPtr<JSStringRef>()". > + wstring text(textBSTR, SysStringLen(textBSTR)); > + SysFreeString(textBSTR); > + JSRetainPtr<JSStringRef> markerText(Adopt, JSStringCreateWithCharacters(text.data(), text.length())); > + return markerText; > } Again, you can use JSStringCreateWithBSTR to make this easier and cleaner.
(In reply to comment #4) > (From update of attachment 61605 [details]) > > +HRESULT WebFrame::elementFromJS(JSContextRef context, JSValueRef nodeObject, IDOMElement **element) > > +{ > > + if (!element) > > + return E_POINTER; > > + > > + *element = 0; > > + > > + if (!context) > > + return E_FAIL; > > + > > + JSLock lock(SilenceAssertionsOnly); > > + ExecState* execState = toJS(context); > > + if (!nodeObject) > > + return E_FAIL; > > I'd null-check nodeObject at the same time as context. Will change. > > > + JSValue jsValue = toJS(execState, nodeObject); > > I don't think the execState or jsValue local variables are useful. Will remove these local variables and write "Element* elt = toElement(jsValue);" as "Element *elt = toElement(toJS(toJS(context), value));". > > > @@ -114,4 +114,6 @@ interface IWebFramePrivate : IUnknown > > HRESULT paintScrollViewRectToContextAtPoint([in] RECT rect, [in] POINT pt, [in] OLE_HANDLE deviceContext); > > > > HRESULT renderTreeAsExternalRepresentation([in] BOOL forPrinting, [out, retval] BSTR* result); > > + > > + [local] HRESULT elementFromJS([in] JSContextRef context, [in] JSValueRef nodeObject, [out, retval] IDOMElement** element); > > Putting this on IWebViewPrivate would be a little more conventional, I think. Will move to IWebViewPrivate. > > > + if (!framePrivate) > > + return JSRetainPtr<JSStringRef>(); > > You can say "return 0" instead of "return JSRetainPtr<JSStringRef>()". > > > + wstring text(textBSTR, SysStringLen(textBSTR)); > > + SysFreeString(textBSTR); > > + JSRetainPtr<JSStringRef> markerText(Adopt, JSStringCreateWithCharacters(text.data(), text.length())); > > + return markerText; > > } > > Again, you can use JSStringCreateWithBSTR to make this easier and cleaner. Will change.
(In reply to comment #5) > Will remove these local variables and write "Element* elt = toElement(jsValue);" as "Element *elt = toElement(toJS(toJS(context), value));". > I meant write as "Element* elt = toElement(toJS(toJS(context), nodeObject));"
Created attachment 61739 [details] Patch Updated patch based on Adam Roben's comments.
Attachment 61739 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/3352713
Comment on attachment 61739 [details] Patch r=me
Committed r63710: <http://trac.webkit.org/changeset/63710>