Opened 11 years ago

Closed 11 years ago

#5922 closed defect (fixed)

Firebug Lite Patch - Multiple Fixes, One Upgrade

Reported by: guest Owned by: Adam Peller
Priority: high Milestone: 1.1
Component: General Version: 1.0
Keywords: firebug lite, debug Cc: mike@…
Blocked By: Blocking:

Description (last modified by Adam Peller)

Fixed Safari: console body did not display:
HTML needs a height in Safari

Fixed #4644 - IE Crashes when clearing console:
IE didn't crash until you hovered over the main page, so it may have been ellusive. Who would have guessed - setting the consoleBody.innerHTML="" fubar'd IE. destroying elements instead.

Fixed General: 100% height on consoleBody, so resizing window will resize contents.

Fixed General: Checking for pop-up blockers, and alerting the user if encountered.

Upgrade: Added cookie code to remeber window's position and size (a much needed feature, I was always very frustrated that it didn't).

Mike Wilcox mike -AT- sitepen -dot- com

Attachments (7)

firebug.patch (6.8 KB) - added by guest 11 years ago.
firebug.2.patch (5.6 KB) - added by guest 11 years ago.
firebug patch #2
firebug.3.patch (5.6 KB) - added by guest 11 years ago.
firebug patch #2
firebug.5.patch (4.9 KB) - added by Adam Peller 11 years ago.
updated now that the console clear part of the patch has been incorporated. still has bug with console key entry.
firebug.4.patch (7.0 KB) - added by guest 11 years ago.
firebug.6.patch (7.1 KB) - added by guest 11 years ago.
File SIX! Fixed a bug anyway
firebug.7.patch (6.8 KB) - added by guest 11 years ago.
Number 7!

Download all attachments as: .zip

Change History (22)

Changed 11 years ago by guest

Attachment: firebug.patch added

comment:1 Changed 11 years ago by guest

Replaced the patch with two minor tweaks: Object Inspector also needed 100% applied to it, and added line break to end of a printed object, to keep a long object from going to the edge of the window.

Mike

comment:2 Changed 11 years ago by Adam Peller

Description: modified (diff)
Milestone: 1.1
Owner: changed from anonymous to Adam Peller
Priority: normalhigh

Thanks for the patch. Some questions:

  • What's "dojo.loaderParseError" and where is it used?
  • The patch appears to make a global out of the function openWin() that was otherwise hidden in a closure. Was there a reason for this?
  • For the cookie, I'd suggest that rather than duplicating the dojo.cookie code, we only take what we need and inline it. In this case, document.cookie= seems sufficient to set... Are the props even needed? getting a cookie is a regexp (see new dojo.cookie code)

If you can get me a minimal patch for the IE crash, I'll get it in for 1.1, and we can do the rest for 1.2.

comment:3 Changed 11 years ago by Adam Peller

nm, I see the change relating to the IE crash and would be happy to integrate it immediately, except I cannot recreate the crash (and I know I've seen it in the past) Can you still make it happen with the current build? Does it matter which page you view or which version of IE is used? I hover hover everything and it makes no difference.

comment:4 Changed 11 years ago by guest

I would submit a test for the crash, but there's nothing spcial about it. Yes, I updated the latest SVN build, and it still crashd. It was the innerHTML, so it's a small and specific fix. It happens on IE7 XP SP2, and is very consistent. popup window on in the page.

Sorry about the "loaderParseError". That is another patch I'm working on that is currently failing in IE - for the Syntax Error Catcher. We moved a majority of teh code here to save bytes in the loader.js. It shouldn't be in here.

I'll look into the global openWin. I certainly didn't mean to do that. Force of habit coding that way.

I'll fix the cookie. I originally didn't see much space savings to change it, but I guess even 4 or 5 lines is a savings. A new patch will be along shortly. We can get all the changes in.

comment:5 Changed 11 years ago by guest

Ugh. Should have previewed that. IE crashes both with an inline Firebug and a popup Firebug. iirc, it also didn't clear the window.

Changed 11 years ago by guest

Attachment: firebug.2.patch added

firebug patch #2

Changed 11 years ago by guest

Attachment: firebug.3.patch added

firebug patch #2

comment:6 Changed 11 years ago by guest

New Patch - use the third one (left debugging in the second). Ignore the link to ticket #2 :)

Mike

comment:7 Changed 11 years ago by Adam Peller

here's what made the crash even more elusive: it only happens over http!

comment:8 Changed 11 years ago by Adam Peller

(In [12609]) Fix IE crash (thanks Mike Wilcox), change deprecated/experimental to console.warn. Fixes #5490 Refs #5922 !strict

Changed 11 years ago by Adam Peller

Attachment: firebug.5.patch added

updated now that the console clear part of the patch has been incorporated. still has bug with console key entry.

comment:9 Changed 11 years ago by Adam Peller

Mike, I integrated the clear crash part, but when I tried the whole patch, I noticed that I couldn't type in the console anymore. Attaching an updated patch which still has this bug.

Changed 11 years ago by guest

Attachment: firebug.4.patch added

comment:10 Changed 11 years ago by guest

Hmmm...That came out as patch.4, and I forgot to add a title to it. Oh well, patch 4!

This fix was a little more extensive (aka, xxx lines of code). 100% height is always a pita because browsers don't believe you should have that ability.

For this to work perfectly, it would need a quite extensive rewrite and put it into a table. I didn't want to go that far, so the accuracy is a smidge off.

I'm grabbing the viewport size and sending the height to layout() onResize. It seems to work pretty well.

comment:11 Changed 11 years ago by Adam Peller

I posted patch "5" (patch 4 on my machine was merged with the latest source to make patch 5 :) I'm not sure your latest patch includes these changes or will apply on the trunk? Does this allow for input?

Changed 11 years ago by guest

Attachment: firebug.6.patch added

File SIX! Fixed a bug anyway

comment:12 Changed 11 years ago by guest

Added another one. There were a few errors that got by me anyway. This is tested and should work. I hope it merges okay, I'm not sure what's going on there.

Mike

comment:13 Changed 11 years ago by Adam Peller

mike, the merge thing is about getting it to run off the latest. I modified the cookie code, removed some formatting changes which messed up indentations, etc. The two patches will need to be merged, I guess.

Changed 11 years ago by guest

Attachment: firebug.7.patch added

Number 7!

comment:14 Changed 11 years ago by guest

OK, I got it. It only took a couple of hours. I updated then made the patch. Hope this is what was needed. If not, I'm gonna need more hand holding.

7 up!

comment:15 Changed 11 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [12640]) Apply Mike Wilcox's firebug patch. 7 up. Fixes issues with Safari, popup blocker, resizing. Adds cookie support to remember position and size. Fixes #5922 !strict

Note: See TracTickets for help on using tickets.