Closed
Bug 331828
Opened 18 years ago
Closed 18 years ago
Can't inspect windows on Mac
Categories
(Other Applications :: DOM Inspector, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9alpha
People
(Reporter: philor, Assigned: philor)
References
Details
(Keywords: dogfood, regression)
Attachments
(1 file, 1 obsolete file)
3.46 KB,
patch
|
Details | Diff | Splinter Review |
Bug 109481 broke inspecting chrome on the Mac: View > Chrome makes the open windows show up in the File > Inspect Document menu, but selecting one doesn't do anything at all: no errors, no actions.
Comment 1•18 years ago
|
||
Can you start the app from the terminal and see if anything's output there?
Assignee | ||
Comment 2•18 years ago
|
||
Oh, there was a reason why I was updating my debug build, wasn't there? Nothing but a string of six WARNING: Not implemented: file /Users/philringnalda/moz/mozilla/widget/src/mac/nsMenuX.cpp, line 403
Comment 3•18 years ago
|
||
In addition to windows, I can't inspect documents by selecting them from the "Inspect Document" menu, either. Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060402 Firefox/1.6a1 ID:2006040301
Flags: blocking1.9a1?
Comment 4•18 years ago
|
||
We need to investigate this more. Any debug output from the console that can help pinpoint this is useful.
Comment 5•18 years ago
|
||
Attachment 207152 [details] [diff] changed the Inspect menu to load/unload lazily when onpopupshowing/onpopuphiding are fired. When the menu hides, it removes each item in the submenu but the mac menu widget code doesn't support this. Thus the assertion in comment 2. Still investigating this...
Comment 6•18 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060517 Minefield/3.0a1 Formerly I could choose: File > Inspect a window, and then I got information about the chrome, but now when I choose File > Inspect a document, I don't see that happen. Nothing happens in fact. See no errors in the JSConsole. The regression range points to bug 109481 as the culprit. The branch DOMI still works fine.
Assignee | ||
Comment 7•18 years ago
|
||
Ria: see comment 0. View menu, Chrome, to toggle including chrome windows in the File menu's list. Though an item at the bottom of the list itself that toggles between "Include chrome windows" and "Hide chrome windows" would probably be better UI.
Comment 8•18 years ago
|
||
(In reply to comment #7) > OK thanks :)
Assignee | ||
Comment 9•18 years ago
|
||
So, what are we going to do here? DOMi's now essentially unusable on the Mac on trunk, and has been for almost three months, and now that the days of freely checking everything into trunk and branch are over, it's only a matter of time before Firefox has something broken Mac-only that can't be inspected. Personally, I don't think the UI benefits from bug 109481 were that great, to abandon one platform for them.
Severity: major → blocker
Comment 10•18 years ago
|
||
If the problem is indeed related to comment 2, it seems relatively minor and must've been encountered before somewhere else - we need to remove the menu items all at once rather than one at a time. Not having a Mac, though, there's no way for me to make/test a patch.
Comment 11•18 years ago
|
||
To see if the menu item removal really is the problem, could someone with a Mac try removing the onpopuphiding attribute here? http://lxr.mozilla.org/seamonkey/source/extensions/inspector/resources/content/popupOverlay.xul#32
Assignee | ||
Comment 12•18 years ago
|
||
Yeah, lxring appendChild(menu and looking at how they cleared the previous menu already led me to clearing onpopupshowing rather than onpopuphiding, so I have a working build, just need to clean up the patch and test on all three platforms.
Assignee: dom-inspector → philringnalda
Comment 13•18 years ago
|
||
Someone ought to file a bug on the underlying issue here though with a testcase of the XUL that is non-working on mac (due to mac's special menu code)
Assignee | ||
Comment 14•18 years ago
|
||
Attachment #226481 -
Flags: review?(timeless)
Comment 15•18 years ago
|
||
Comment on attachment 226481 [details] [diff] [review] fix >+ <menupopup id="listDocuments-popup" onpopupshowing="inspector.showInspectDocumentList()"/> Nit: Maybe put the onpopupshowing attribute on the next line, to keep each line under 80 characters. Looks good to me, though.
Comment 16•18 years ago
|
||
Comment on attachment 226481 [details] [diff] [review] fix >+ // clear out existing menu I don't see the value of in-lining this code. >+ var popup = this.mInspectDocumentMenu; >+ while (popup.firstChild) { >+ popup.removeChild(popup.firstChild); >+ } Using the out of line function gives a better indication to me about what's going on. but i don't feel that strongly about it. as for the 80 cols, i accept that you weren't really changing that line, but unbunching it to two rows would probably be slightly better.
Attachment #226481 -
Flags: superreview?(neil)
Attachment #226481 -
Flags: review?(timeless)
Attachment #226481 -
Flags: review+
Comment 17•18 years ago
|
||
Comment on attachment 226481 [details] [diff] [review] fix >+ // clear out existing menu Logically I feel this code belongs at line 405-410 instead. >+ while (popup.firstChild) { If you're going to change the test I would suggest popup.hasChildNodes() >+ popup.removeChild(popup.firstChild); The old code popup.removeChild(popup.lastChild); is slightly more efficient.
Attachment #226481 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 18•18 years ago
|
||
Not inlined, called in a more reasonable place, with hasChildNodes() and killing the last, youngest and weakest child first, and under 80 cols. Thanks for the comments!
Attachment #226481 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Whiteboard: [checkin needed]
Comment 19•18 years ago
|
||
mozilla/extensions/inspector/resources/content/popupOverlay.xul 1.15 mozilla/extensions/inspector/resources/content/inspector.js 1.28
Status: NEW → RESOLVED
Closed: 18 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
Comment 20•18 years ago
|
||
Does this bug exist on the branch? If so, we should definitely nominate it as blocker, since DOM inspector is hardly usable with this bug...
Assignee | ||
Comment 21•18 years ago
|
||
No, bug 109481 didn't touch branch, so we're fine.
Updated•17 years ago
|
QA Contact: timeless → dom-inspector
You need to log in
before you can comment on or make changes to this bug.
Description
•