224164 - inCSSValueSearch shouldn't use internal APIs

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[David Baron :dbaron:]

inCSSValueSearch shouldn't use internal APIs.  This causes pain such as the
bustage from bug 167262's codesize reduction checkin and the existence of
nsICSSStyleRule::GetValue and the things it calls.  I have a preliminary patch
(untested), but it needs some work in inCSSValueSearch::SearchStyleValue.

Comment 1

[David Baron :dbaron:]

Attachment: partial patch

Comment 2

[David Baron :dbaron:]

caillon, how do I test this code?  The only caller seems to be
http://lxr.mozilla.org/seamonkey/source/extensions/inspector/resources/content/search/modules/junkImgs/junkImgs.xml

Comment 3

[David Baron :dbaron:]

Attachment: patch

This should be equivalent to the old code.

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

> caillon, how do I test this code? 

You probably don't.  This looks like another of hewitt's unfinished pieces...

Comment 5

[David Baron :dbaron:]

Comment on attachment 137807 [details] [diff] [review]
patch

In that case, I think I'd like to just get it reviewed, then, since I'd rather
not have to go through more pain because this code is depending on unstable
APIs...

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 137807 [details] [diff] [review]
patch

>-  if (value.GetUnit() == eCSSUnit_URL) {
>+  if (StringBeginsWith(aValue, NS_LITERAL_STRING("url(")) &&
>+      StringEndsWith(aValue, NS_LITERAL_STRING(")"))) {

I tried thinking of a way to use nsIDOMCSSPrimitiveValue::CSS_URL, but there
really isn't one without knowledge of the css parser (in more places).	Oh,
well.

r=caillon

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 137807 [details] [diff] [review]
patch

>Index: inCSSValueSearch.cpp

>+inCSSValueSearch::SearchStyleSheet(nsIDOMCSSStyleSheet* aStyleSheet, nsIURI* aBaseURL)

>+  if (aStyleSheet) {

Just return early if that's null, maybe?

>+      NS_NewURI(getter_AddRefs(baseURL), NS_ConvertUTF16toUTF8(href),
>+                nsnull, aBaseURL);

drop the NS_ConvertUTF16toUTF8 -- nsNetUtil has a NS_NewURI that takes an
nsAString.   It'll compile to the same code if it gets inlined like it's
supposed to, but will be more readable...

>+inCSSValueSearch::SearchStyleRule(nsIDOMCSSStyleRule* aStyleRule, nsIURI* 

>+    decl->GetPropertyValue(property, value);

This could use an XXX comment saying that GetPropertyCSSValue is what we really
want here... once we implement it.

sr=bzbarsky with that.

Comment 8

[David Baron :dbaron:]

Fix checked in to trunk, 2003-12-29 11:05 -0800.