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 )
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)
Change History (22)
Changed 11 years ago by
| Attachment: | firebug.patch added |
|---|
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
| Description: | modified (diff) |
|---|---|
| Milestone: | → 1.1 |
| Owner: | changed from anonymous to Adam Peller |
| Priority: | normal → high |
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
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
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
Ugh. Should have previewed that. IE crashes both with an inline Firebug and a popup Firebug. iirc, it also didn't clear the window.
comment:6 Changed 11 years ago by
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
here's what made the crash even more elusive: it only happens over http!
comment:8 Changed 11 years ago by
Changed 11 years ago by
| 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
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
| Attachment: | firebug.4.patch added |
|---|
comment:10 Changed 11 years ago by
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
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?
comment:12 Changed 11 years ago by
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
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.
comment:14 Changed 11 years ago by
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
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
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