Closed
Bug 612861
Opened 14 years ago
Closed 14 years ago
Some content is missing on www.msnbc.com, gmail reply is broken
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla2.0b8
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta8+ |
People
(Reporter: streetwolf52, Assigned: Gavin)
References
()
Details
(Keywords: regression)
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101116 Firefox/4.0b8pre
Build Identifier: 20101116135406
Some content is missing on www.msnbc.com. The main header is missing, the local weather, Stock information, and probably some others.
Reproducible: Always
Regression range:
http://hg.mozilla.org/mozilla-central/rev/898ef162e026 - Bad
http://hg.mozilla.org/mozilla-central/rev/ef83567ee8d8 - Good
This appears in my error log with the 'bad' build:
Error: Permission denied to access property 'firebug'
Source File: http://www.msnbc.msn.com/
Line: 922
Error: Permission denied to access property 'firebug'
Source File: http://www.msnbc.msn.com/id/23297925
Line: 4
Error: DBMgr is undefined
Source File: http://www.msnbc.msn.com/
Line: 1534
Error: DBMgr is undefined
Source File: http://www.msnbc.msn.com/id/23191536
Line: 1
Error: Permission denied to access property 'firebug'
Source File: http://www.msnbc.msn.com/id/23284290?LNW.js
Line: 1
Error: DBMgr is undefined
Source File: http://www.msnbc.msn.com/
Line: 1916
Error: DBMgr is undefined
Source File: http://www.msnbc.msn.com/id/35109355
Line: 2
Reporter | ||
Updated•14 years ago
|
Keywords: regression
Version: unspecified → Trunk
Comment 1•14 years ago
|
||
> Error: Permission denied to access property 'firebug'
The relevant source line is:
if (!("console" in window) || !("firebug" in console)) {
The checkin range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ef83567ee8d8&tochange=898ef162e026
which is when we started adding our own console object to windows. Sounds like it's being created in such a way that random property gets throw, which is broken. That needs to be fixed.
Assignee: general → nobody
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Component: JavaScript Engine → Developer Tools
Ever confirmed: true
Product: Core → Firefox
QA Contact: general → developer.tools
Comment 2•14 years ago
|
||
ccing the code author too.
Comment 3•14 years ago
|
||
and adding jst who consulted and provided code.
Comment 4•14 years ago
|
||
why would we be getting a permission denied error accessing that property? I see that too in today's build, just running | "firebug" in console | in the console's input line.
Comment 5•14 years ago
|
||
(In reply to comment #1)
> > Error: Permission denied to access property 'firebug'
>
> The relevant source line is:
>
> if (!("console" in window) || !("firebug" in console)) {
>
11:11:18.633: Exception: Permission denied to access property 'firebug' Source File: http://www.msnbc.msn.com/ Line: 922, Column: 0 Category: content javascript
11:11:18.662: Exception: Permission denied to access property 'firebug' Source File: http://www.msnbc.msn.com/id/23297925 Line: 4, Column: 0 Category: content javascript
11:11:18.665: Exception: DBMgr is undefined Source File: http://www.msnbc.msn.com/ Line: 1548, Column: 0 Category: content javascript
(from the console)
That ("firebug" in console) check is a fairly common pattern to try to identify if Firebug's on a given browser. Looks like they're using this check to then null out the console object to "hide" from Firebug.
There's good testcase fodder in here.
Comment 6•14 years ago
|
||
Somewhat related to this, is the current plan to always provide the console object even if the console isn't being used? Seems like pages could get into a slower "debug" or "developer mode" this way.
Comment 7•14 years ago
|
||
not really the best place to discuss this, dao, but the "lazy console" will produce a console object when it's asked for.
The next stage is create a service to capture logging information to display in the hudservice.
see bug 612658 and follow up there with questions.
Comment 8•14 years ago
|
||
(In reply to comment #7)
> not really the best place to discuss this, dao, but the "lazy console" will
> produce a console object when it's asked for.
I was asking because if the console object wasn't always there in the first place, this bug wouldn't affect all users...
> The next stage is create a service to capture logging information to display in
> the hudservice.
>
> see bug 612658 and follow up there with questions.
My point is that developers right now don't expect the console object to exist for ordinary users, so they could kick off crazy debug tasks beyond just logging simple strings. Bug 612658 doesn't really seem relevant in this context (as in, it won't help).
Updated•14 years ago
|
Comment 9•14 years ago
|
||
Dão, there's always a console object present in Opera and all webkit-based browsers, for what it's worth. Not sure about IE. But we've had pages break because developers assumed a console object _is_ always present.....
Comment 10•14 years ago
|
||
(In reply to comment #9)
> Dão, there's always a console object present in Opera and all webkit-based
> browsers, for what it's worth.
Ok, I'll be quiet then.
Comment 11•14 years ago
|
||
This appears to be sandbox related, i think. Overwriting "window.console" is not the same as overwriting "console"
Assignee | ||
Comment 12•14 years ago
|
||
The __exposedProps__ thing is what causes the random property gets to fail, I think. We can probably just avoid using it.
Comment 13•14 years ago
|
||
(In reply to comment #12)
> The __exposedProps__ thing is what causes the random property gets to fail, I
> think. We can probably just avoid using it.
I used __exposedProps__ so we would only be exposing the API. I would think it better if content cannot access our internal methods.
Comment 14•14 years ago
|
||
Assignee: nobody → rcampbell
Assignee | ||
Updated•14 years ago
|
blocking2.0: ? → beta8+
Assignee | ||
Comment 15•14 years ago
|
||
Assignee: rcampbell → gavin.sharp
Attachment #491237 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #491241 -
Flags: review?(bzbarsky)
Attachment #491241 -
Flags: feedback?
Assignee | ||
Updated•14 years ago
|
Attachment #491241 -
Flags: feedback? → feedback?(rcampbell)
Assignee | ||
Comment 16•14 years ago
|
||
Comment on attachment 491241 [details] [diff] [review]
patch
Please hold, this is buggy.
Attachment #491241 -
Attachment is obsolete: true
Attachment #491241 -
Flags: review?(bzbarsky)
Attachment #491241 -
Flags: feedback?(rcampbell)
Assignee | ||
Comment 17•14 years ago
|
||
Attachment #491244 -
Flags: review?(bzbarsky)
Attachment #491244 -
Flags: feedback?
Assignee | ||
Updated•14 years ago
|
Attachment #491244 -
Flags: feedback? → feedback?(rcampbell)
Assignee | ||
Updated•14 years ago
|
Component: Developer Tools → DOM
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: developer.tools → general
Hardware: x86_64 → All
Assignee | ||
Comment 18•14 years ago
|
||
Of course, this will break the "console replaced" warning. Sigh! I suppose I should just add |classID: self.classID| to the returned object for the moment, pending a fix for bug 612405.
Comment 19•14 years ago
|
||
Comment on attachment 491244 [details] [diff] [review]
patch
this looks good.
Attachment #491244 -
Flags: feedback?(rcampbell) → feedback+
Assignee | ||
Comment 20•14 years ago
|
||
With comment 18 addressed. Sorry for the patch churn...
Attachment #491244 -
Attachment is obsolete: true
Attachment #491253 -
Flags: review?(bzbarsky)
Attachment #491244 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 21•14 years ago
|
||
Attachment #491253 -
Attachment is obsolete: true
Attachment #491254 -
Flags: review?(bzbarsky)
Attachment #491253 -
Flags: review?(bzbarsky)
Comment 22•14 years ago
|
||
I don't think there's any problem with "leaking" the classID, fwiw. Pretty low on the danger scale.
Comment 23•14 years ago
|
||
Comment on attachment 491254 [details] [diff] [review]
[checked-in] patch
r=me
Attachment #491254 -
Flags: review?(bzbarsky) → review+
Replying to mail in gmail fails with the same error message.
Summary: Some content is missing on www.msnbc.com. → Some content is missing on www.msnbc.com, gmail reply is broken
Comment 25•14 years ago
|
||
ran a full set of mochitests here and tested the patch manually on msnbc and gmaps. Looks good to go. Ready to land...
Comment 26•14 years ago
|
||
Comment on attachment 491254 [details] [diff] [review]
[checked-in] patch
http://hg.mozilla.org/mozilla-central/rev/f31dab55e41a
Attachment #491254 -
Attachment description: patch → [checked-in] patch
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•14 years ago
|
Target Milestone: --- → mozilla2.0b8
Comment 28•14 years ago
|
||
(In reply to comment #9)
> Dão, there's always a console object present in Opera and all webkit-based
> browsers, for what it's worth. Not sure about IE. But we've had pages break
> because developers assumed a console object _is_ always present.....
FWIW, I think that should be fixed and filed bug 606793 on that one.
Comment 31•14 years ago
|
||
Gmail broken in different ways today:
https://bugzilla.mozilla.org/show_bug.cgi?id=613153
Related?
Comment 32•14 years ago
|
||
investigating
Comment 33•14 years ago
|
||
Ed, I think this is unrelated.
In your about:config, try disabling methodjit.
javascript.options.methodjit.chrome = false.
Comment 34•14 years ago
|
||
see bug 613153.
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•