[Webkit-unassigned] [Bug 37930] [Win] Implement LayoutTestController::markerTextForListItem()

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Daniel Bates <dbates at webkit.org>  2010-07-15 13:15:55 PST ---
(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.

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