547304 - “Allow Flash From This Site” missing from context menu in 1.9.2

Status: RESOLVED FIXED

(Camino Graveyard :: Toolbars & Menus, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

When invoking a context menu on a Flashblock <div>, the “Allow Flash From This Site” item is missing from the context menu on 1.9.2.

This worked on 1.9.1, so it's something new to 1.9.2.

Comment 1

[Christopher Henderson]

Attachment: Fix v1.0

Can't seem to get an nsIDOMCSSPrimitiveValue from the element's CSS value, so I just get the CSS value directly from the nsIDOMCSSValue.

Comment 2

[Christopher Henderson]

Attachment: Fix v1.1

This is the same as version 1.0, but with more Gecko macro goodness.

Comment 3

[Stuart Morgan]

Comment on attachment 432646 [details] [diff] [review]
Fix v1.1

Please apply the new rule here. Specifically, check:
docView
defaultCSSView
computedStyle

Comment 4

[Christopher Henderson]

Attachment: Fix v1.2, for checkin

Added the required null check macros for all pointers that get dereferenced.

Comment 5

[Stuart Morgan]

Comment on attachment 434146 [details] [diff] [review]
Fix v1.2, for checkin

>+  rv = computedStyle->GetPropertyCSSValue(NS_LITERAL_STRING("background-image"),
>+                                          getter_AddRefs(cssValue));
>+  NS_ENSURE_SUCCESS(rv, NO);
>+  NS_ENSURE_TRUE(cssValue, NO);

Do you get success and a non-null pointer for an element that doesn't have a background-image?

>+  rv = cssValue->GetCssText(bgStringValue);
>+  NS_ENSURE_SUCCESS(rv, NO);

And does this return success for an element with no background-image?

If both those things are true, sr=smorgan, otherwise you need to replace them with simple checks, not assertions.

Comment 6

[Christopher Henderson]

Comment on attachment 434146 [details] [diff] [review]
Fix v1.2, for checkin

Yes, it's successful for elements with no bg image. Nothing is logged to the console in that case.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Landed in the test repo: http://hg.mozilla.org/users/alqahira_ardisson.org/camino-1.9.2-test/rev/5c3348c5f2ec

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/3696e1b41214

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Oops, this was the one where I flubbed checkin; the last one was followed by a backout and relanding because I didn't have any better sea legs then:

http://hg.mozilla.org/camino/rev/c8c642f7757b
http://hg.mozilla.org/camino/rev/6eaa9df28af3