Closed Bug 612861 Opened 9 years ago Closed 9 years ago

Some content is missing on www.msnbc.com, gmail reply is broken

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: streetwolf, Assigned: Gavin)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 4 obsolete files)

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
Keywords: regression
Version: unspecified → Trunk
> 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
ccing the code author too.
and adding jst who consulted and provided code.
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.
(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.
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.
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.
(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).
No longer blocks: 612876, 587734
Blocks: 612876, 587734
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.....
(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.
Blocks: devtools4
This appears to be sandbox related, i think. Overwriting "window.console" is not the same as overwriting "console"
The __exposedProps__ thing is what causes the random property gets to fail, I think. We can probably just avoid using it.
(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.
Attached patch fix (obsolete) — Splinter Review
Assignee: nobody → rcampbell
blocking2.0: ? → beta8+
Attached patch patch (obsolete) — Splinter Review
Assignee: rcampbell → gavin.sharp
Attachment #491237 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #491241 - Flags: review?(bzbarsky)
Attachment #491241 - Flags: feedback?
Attachment #491241 - Flags: feedback? → feedback?(rcampbell)
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)
Attached patch patch (obsolete) — Splinter Review
Attachment #491244 - Flags: review?(bzbarsky)
Attachment #491244 - Flags: feedback?
Attachment #491244 - Flags: feedback? → feedback?(rcampbell)
Component: Developer Tools → DOM
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: developer.tools → general
Hardware: x86_64 → All
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 on attachment 491244 [details] [diff] [review]
patch

this looks good.
Attachment #491244 - Flags: feedback?(rcampbell) → feedback+
Attached patch patch (obsolete) — Splinter Review
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)
Attachment #491253 - Attachment is obsolete: true
Attachment #491254 - Flags: review?(bzbarsky)
Attachment #491253 - Flags: review?(bzbarsky)
I don't think there's any problem with "leaking" the classID, fwiw. Pretty low on the danger scale.
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
ran a full set of mochitests here and tested the patch manually on msnbc and gmaps. Looks good to go. Ready to land...
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
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Duplicate of this bug: 613012
Target Milestone: --- → mozilla2.0b8
(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.
Duplicate of this bug: 613135
Duplicate of this bug: 613181
Gmail broken in different ways today:
https://bugzilla.mozilla.org/show_bug.cgi?id=613153

Related?
investigating
Ed, I think this is unrelated.

In your about:config, try disabling methodjit.

javascript.options.methodjit.chrome = false.
Depends on: 613414
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.