My favorites | Sign in
Project Home Wiki Issues
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 4906: Show notification in Script panel if tab cache is exceeded
1 person starred this issue and may be notified of changes. Back to list
Status:  Fixed
Owner:  odva...@gmail.com
Closed:  Sep 2013
Cc:  simon.lindholm10


Sign in to add a comment
 
Reported by sebastia...@gmx.de, Oct 11, 2011
What should be implemented/improved?
In case the tab cache (extensions.firebug.cache.responseLimit) is exceeded show a proper notification, that gives the user the hint, that he can change that limit. A proper message text would be:

"Firebug's cache limit of xxxxx Bytes reached. Limit can be changed via extensions.firebug.cache.responseLimit."

Test case:
1. Open Firebug at https://getfirebug.com/tests/script/4868/issue4868.html 
2. Enable and switch to the Script panel
3. Reload the page

=> A notification should be shown telling the user, that the cache limit was exceeded and he can change this limit via extensions.firebug.cache.responseLimit
Aug 30, 2013
Project Member #1 sebastia...@gmail.com
The test case moved to https://getfirebug.com/tests/manual/script/4868/issue4868.html.

Sebastian
Summary: Show notification if tab cache is exceeded (was: Show notification, if tab cache is exceeded)
Sep 2, 2013
Project Member #2 odva...@gmail.com
(No comment was entered for this change.)
Status: Started
Owner: odva...@gmail.com
Cc: -odva...@gmail.com
Sep 2, 2013
Project Member #3 odva...@gmail.com
Development branch:
https://github.com/firebug/firebug/commits/issue4906

Honza
Sep 2, 2013
Project Member #4 simon.lindholm10
BTW, if you didn't already think of it, we can't directly do this from sourceCache when the limit is exceeded, because that happens frequently for binary files (I guess). Also it would feel confusing and irrelevant. So we need to either have special logic for e.g. Script panel and Net panel, or (possibly) some special flag that we can set when we call sourceCache.load.
Sep 2, 2013
Project Member #5 sebastia...@gmail.com
> that happens frequently for binary files
Seems like I wasn't clear enough in my description. This issue just relates to the Script panel. I.e. we're talking about JavaScript content here. The Net panel should stay untouched by this issue.

Sebastian
Summary: Show notification in Script panel if tab cache is exceeded (was: Show notification if tab cache is exceeded)
Sep 3, 2013
Project Member #6 odva...@gmail.com
> This issue just relates to the Script panel.
Right

List of commits, related to this issue:
https://github.com/firebug/firebug/commit/abaa7c80abe9101125b256fc810308d549ebc4df
https://github.com/firebug/firebug/commit/28d321e3594a59242c6707a72eec0a8718dac2ac
https://github.com/firebug/firebug/commit/4bd6d6f614e16262751fc7add603ca038209b775
https://github.com/firebug/firebug/commit/6db6503cff7106d4ed8548d32e2e666ee36f5ded

Summary:
- NetLimit template renamed to PanelNotification.
- PanelNotification is generic reused by panels (Console, Script, Net)
- CSS for the panel notification template is in panelNotification.css
- The Script panel displays a notification (at the top) if the current script has been limited. The message is: "Script has been limited since it reached the maximum cache entry size." (let me know if you have better wording)
- If the cache limit pref is changed, the browser doesn't have to be restarted, just the page must be reloaded.

Related discussion:
https://groups.google.com/forum/#!topic/firebug/GmeZD4xGLRc

Honza


Status: Commit
Sep 3, 2013
Project Member #7 sebastia...@gmail.com
> "Script has been limited since it reached the maximum cache entry size."
I'm not sure if the tooltip on the button is visible enough.
So what about "The script's content exceeds the maximum cache size. (Current limit is %S).\nSee 'extensions.firebug.cache.responseLimit' preference."
That would be similar to 'console.multiHighlightLimitExceeded'.

Nit: The panel notification border should extend to the edges of the panel. See the attached screenshot.

Besides that, good job!

Sebastian
panelNotificationBorder.png
18.7 KB   View   Download
Sep 3, 2013
Project Member #8 simon.lindholm10
> Script has been limited
This does sound vague to me, and it's also not totally clear that this only affects the Script panel's display. Something like, "Firebug's cache limit has been reached - only showing the first N lines."?
Sep 3, 2013
Project Member #9 odva...@gmail.com
Some typos fixed at:
https://github.com/firebug/firebug/commit/084db43b171025e91b339f43d29a9d0cbbdaf0df

> Nit: The panel notification border should extend to the edges of the panel.
> See the attached screenshot.
I don't see this on my machine, can you play with this number:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/js/scriptPanel.js#L1000

-> this.selectedSourceBox.style.top = "30px";


> "Firebug's cache limit has been reached - only showing the first N lines."?
In theory there can be one limited line only (compressed JS).

> So what about "The script's content exceeds the maximum cache size. (Current limit > is %S).\nSee 'extensions.firebug.cache.responseLimit' preference."
Note that the Console and Net panel also don't show the pref. If we want to show it it should be done for all.

We can discuss on today's meeting.

Honza
Sep 3, 2013
Project Member #10 simon.lindholm10
> Note that the Console and Net panel also don't show the pref. If we want to show it it should be done for all.
The current link to about:config is really undiscoverable, so either we should do that, or we should link directly to about:config?filter=extensions.firebug.cache.responseLimit.

> In theory there can be one limited line only (compressed JS).
Yeah, the "N lines" thing doesn't work fully (without something like special-casing single-line files, and removing the last unfinished line). But we can say something similar.
Sep 3, 2013
Project Member #11 sebastia...@gmail.com
> The current link to about:config is really undiscoverable, so either we should do 
> that, or we should link directly to about:config?filter=extensions.firebug.cache.responseLimit.
Didn't know about that possibility. We should definitely add that. Then it's fine for me to keep the text as it is.

>> Nit: The panel notification border should extend to the edges of the panel.
>> See the attached screenshot.
> I don't see this on my machine, can you play with this number:
> https://github.com/firebug/firebug/blob/master/extension/content/firebug/js/scriptPanel.js#L1000
> 
> -> this.selectedSourceBox.style.top = "30px";
Used the computed height of the panel notification instead of a fixed value in https://github.com/firebug/firebug/commit/73cdc9fdc2db725e6b58f1f16fabc7b65d6d1fe6. Still not perfect, though, because changing the text size still causes display problems.

Sebastian
Sep 3, 2013
Project Member #12 simon.lindholm10
> Didn't know about that possibility.
Yeah, it's hidden to say the least. I don't entirely remembered how I found out about it.

How about "Firebug's cache limit has been reached. Parts of the file are not shown." or so?
Sep 3, 2013
Project Member #13 sebastia...@gmail.com
> How about "Firebug's cache limit has been reached. Parts of the file are not shown." or so?
A bit more precise: "Firebug's cache limit has been exceeded. Parts of the script are not shown."

Also we may want to change the button caption from "Preferences" to something like "Change the limit...", so it's easier for people to understand why they should click the button.

Sebastian
Cc: simon.lindholm10
Sep 3, 2013
Project Member #14 odva...@gmail.com
> or we should link directly to
> about:config?filter=extensions.firebug.cache.responseLimit.
Nice!

> "Firebug's cache limit has been exceeded. Parts of the script are not shown."
I like it

Both fixed at:
https://github.com/firebug/firebug/commit/7944796ef246fb665a1116cc657fefa5e4876725

> Still not perfect, though, because changing the text size still causes display problems.
A soon as CodeMirror is in place the problem will be gone.

> from "Preferences" to something like "Change the limit..."
I believe "Preferences" is good enough 

Thanks!

Honza

Sep 6, 2013
Project Member #15 odva...@gmail.com
This issue has been fixed in Firebug 1.13 alpha 1
https://getfirebug.com/releases/firebug/1.13/firebug-1.13.0a1.xpi

Please let us know if it works for you.

Thanks for the help!

Honza
Status: Fixed
Labels: fixed-1.13-a1
Sign in to add a comment

Powered by Google Project Hosting