The default bug view has changed. See this FAQ.

XRayWrapper does not handle new bindings constants

RESOLVED FIXED in Firefox 15

Status

()

Core
XPConnect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: khuey, Assigned: peterv)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(firefox14 affected, firefox15+ fixed, firefox16+ fixed, firefox17+ fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments, 2 obsolete attachments)

XRayWrapper doesn't pass through constants properly on new bindings.
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
tracking-firefox17: --- → ?

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 774152

Updated

5 years ago
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Updated

5 years ago
Duplicate of this bug: 774152
(Assignee)

Comment 3

5 years ago
Created attachment 643134 [details] [diff] [review]
v1
Assignee: nobody → peterv
Status: REOPENED → ASSIGNED
(Assignee)

Comment 4

5 years ago
Created attachment 643151 [details] [diff] [review]
v1.1
Attachment #643134 - Attachment is obsolete: true
Attachment #643151 - Flags: review?(bzbarsky)

Updated

5 years ago
Blocks: 773460
Created attachment 643209 [details] [diff] [review]
Refactoring I did on the way to maybe fixing this bug, in case we ever want it
Comment on attachment 643151 [details] [diff] [review]
v1.1

s/constanst/constants/ and s/Otherwis/Otherwise/

r=me, though maybe we should consider reducing codesize by refactoring this into a function like in the diff I attached...
Attachment #643151 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 7

5 years ago
Created attachment 643277 [details] [diff] [review]
Refactoring v1.1
Attachment #643209 - Attachment is obsolete: true
Attachment #643277 - Flags: review?(bzbarsky)
Comment on attachment 643277 [details] [diff] [review]
Refactoring v1.1

r=me.  I take it you were ok with the parts of the code I'd already written.  ;)
Attachment #643277 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/03a0d026fca1
https://hg.mozilla.org/integration/mozilla-inbound/rev/37898d7d1189
Tracking for 15 and up, but I'm curious what the user impact to our 14 users is.
tracking-firefox15: ? → +
tracking-firefox16: ? → +
tracking-firefox17: ? → +
This might break addons, but given that we only converted xhr its unlikely.  It's worse now on trunk because we converted canvas on certain platforms.  That broke e.g. out of process reftest and is much more likely to break addons.
It only broke Azure Canvas which is prefed off by default as it is still in development. This bug is blocking turning Azure Canvas on.

Comment 13

5 years ago
(In reply to Anthony Jones (:kentuckyfriedtakahe) from comment #12)
> It only broke Azure Canvas which is prefed off by default as it is still in
> development. This bug is blocking turning Azure Canvas on.

To clarify, it breaks Azure/Cairo canvas which has not yet landed. Azure/D2D canvas is on by default for Windows, I am not sure if this breaks it or how thoroughly it is being tested - Bas?
This bug affects the Azure canvas on Windows.  It only 'breaks' accessing constants on a content object from chrome through.
That bug you mention is for the Azure-cairo canvas.

If you actually look at the gfx.canvas.azure.enabled pref, it defaults to true right now, so you get Azure canvas if AzureCanvasEnabledOnPlatform is true.  Which it is on Linux, Android Mac, and on Windows if either direct2d or skia is being used as the rendering backend.

So basically, if you're not on WinXP you're using Azure canvas today.
And yes, as comment 14 says the scope of the problem is pretty limited.

Comment 17

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #15)
> That bug you mention is for the Azure-cairo canvas.
> 
> If you actually look at the gfx.canvas.azure.enabled pref, it defaults to
> true right now, so you get Azure canvas if AzureCanvasEnabledOnPlatform is
> true.  Which it is on Linux, Android Mac, and on Windows if either direct2d
> or skia is being used as the rendering backend.
> 
> So basically, if you're not on WinXP you're using Azure canvas today.

Hmm, that is a bit scary, Azure/Skia canvas is not meant to be on anywhere by default :-s I'll look into it, but I guess this bug is not the right place for discussing it.
> Azure/Skia canvas is not meant to be on anywhere by default

It's not, because Skia is not on by default.
https://hg.mozilla.org/mozilla-central/rev/03a0d026fca1
https://hg.mozilla.org/mozilla-central/rev/37898d7d1189
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Updated

5 years ago
status-firefox17: affected → fixed
(Assignee)

Comment 20

5 years ago
Comment on attachment 643151 [details] [diff] [review]
v1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 740069
User impact if declined: could break addons (we didn't get any reports yet)
Testing completed (on m-c, etc.): landed on mozilla-central, has a test
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #643151 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 21

5 years ago
Comment on attachment 643277 [details] [diff] [review]
Refactoring v1.1

See comment 20
Attachment #643277 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 22

5 years ago
Does anyone think we should take this on beta?
I think it's probably a good idea.  We did the WebGL context in 15, and that has a bunch of constants on it....
(Assignee)

Updated

5 years ago
Attachment #643151 - Flags: approval-mozilla-beta?
(Assignee)

Updated

5 years ago
Attachment #643277 - Flags: approval-mozilla-beta?
Comment on attachment 643151 [details] [diff] [review]
v1.1

[Triage Comment]
Low risk fix that will keep us from scratching our heads when users report new add-on regressions. Approved for Aurora 16 and Beta 15.
Attachment #643151 - Flags: approval-mozilla-beta?
Attachment #643151 - Flags: approval-mozilla-beta+
Attachment #643151 - Flags: approval-mozilla-aurora?
Attachment #643151 - Flags: approval-mozilla-aurora+

Updated

5 years ago
Attachment #643277 - Flags: approval-mozilla-beta?
Attachment #643277 - Flags: approval-mozilla-beta+
Attachment #643277 - Flags: approval-mozilla-aurora?
Attachment #643277 - Flags: approval-mozilla-aurora+
Er... Peter, does that test fail without this patch?  I don't see why this sandbox would be using Xrays, exactly...
Nevermind.  It defaults to Xrays because of the about:blank bits...  I'll just add a test to make sure it's really got Xrays.
(Assignee)

Comment 27

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/c98bb360201d
https://hg.mozilla.org/releases/mozilla-aurora/rev/cc561d1aa5a6
https://hg.mozilla.org/releases/mozilla-beta/rev/984e5eee5eea
https://hg.mozilla.org/releases/mozilla-beta/rev/533ea5877be6
status-firefox15: affected → fixed
status-firefox16: affected → fixed

Comment 28

4 years ago
Marking as [qa-] since there is a test verifying this bug fix in the push (https://hg.mozilla.org/releases/mozilla-beta/rev/984e5eee5eea).

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.