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

RESOLVED FIXED

Status

Camino Graveyard
Toolbars & Menus
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Smokey Ardisson (offline for a while; not following bugs - do not email), Assigned: Christopher Henderson)

Tracking

1.9.2 Branch
All
Mac OS X

Details

(Whiteboard: [cm192test])

Attachments

(1 attachment, 2 obsolete attachments)

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.
(Assignee)

Comment 1

8 years ago
Created attachment 427860 [details] [diff] [review]
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.
Assignee: nobody → trendyhendy2000
Status: NEW → ASSIGNED
Whiteboard: [1.9.x]
Version: unspecified → 1.9.2 Branch
(Assignee)

Comment 2

8 years ago
Created attachment 432646 [details] [diff] [review]
Fix v1.1

This is the same as version 1.0, but with more Gecko macro goodness.
Attachment #427860 - Attachment is obsolete: true
Attachment #432646 - Flags: superreview?(stuart.morgan+bugzilla)

Comment 3

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

Please apply the new rule here. Specifically, check:
docView
defaultCSSView
computedStyle
Attachment #432646 - Flags: superreview?(stuart.morgan+bugzilla) → superreview-
(Assignee)

Comment 4

8 years ago
Created attachment 434146 [details] [diff] [review]
Fix v1.2, for checkin

Added the required null check macros for all pointers that get dereferenced.
Attachment #432646 - Attachment is obsolete: true
Attachment #434146 - Flags: superreview?(stuart.morgan+bugzilla)

Comment 5

8 years ago
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.
Attachment #434146 - Flags: superreview?(stuart.morgan+bugzilla) → superreview+
(Assignee)

Updated

8 years ago
Attachment #434146 - Attachment description: Fix v1.2 → Fix v1.2, for checkin
(Assignee)

Comment 6

8 years ago
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.
http://hg.mozilla.org/camino/rev/3696e1b41214
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
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
You need to log in before you can comment on or make changes to this bug.