browser.xul.error_pages.enabled should take effect immediately in the same window/tab

RESOLVED FIXED

Status

()

Core
Document Navigation
--
minor
RESOLVED FIXED
13 years ago
5 years ago

People

(Reporter: mcsmurf, Assigned: Vidar Haarr (not reading bugmail))

Tracking

(Blocks: 1 bug)

Trunk
x86
Windows 2000
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

11.63 KB, patch
Vidar Haarr (not reading bugmail)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
When you change the value of browser.xul.error_pages.enabled, it only takes
effect after the next restart. It should take effect immediately.
(Reporter)

Updated

13 years ago
Summary: browser.xul.error_pages.enabled should take effect immediatelly → browser.xul.error_pages.enabled should take effect immediately

Comment 1

13 years ago
WFM (takes effect immediately) with Mozilla/5.0 (Windows; U; Windows NT 5.0;
en-US; rv:1.8b2) Gecko/20050224 Firefox/1.0+

Comment 2

13 years ago
WFM as well:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050224
Firefox/1.0+
er, this works for you two in currently opened windows/tabs?

Comment 4

13 years ago
Hmm actually no, it won't work in the same tab that I change the pref in but if
I open a new tab it works there. That's not what the summary describes though.
Morph the bug?
(Reporter)

Updated

13 years ago
Summary: browser.xul.error_pages.enabled should take effect immediately → browser.xul.error_pages.enabled should take effect immediately in the same window/tab
my understanding is that this bug always was about that problem, but anyway,
let's use it for that one.

Comment 6

13 years ago
A change to "browser.xul.error_pages.enabled" does indeed take effect
immediately.  But note that if you set it to "false", a restart of FF always
returns it to "true".  Apparently "true" is the only acceptable state, and
"false" is only allowed temporarily.

Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.8b2) Gecko/20050316 Firefox/1.0+

Comment 7

13 years ago
(In reply to comment #6)
> A change to "browser.xul.error_pages.enabled" does indeed take effect
> immediately.  But note that if you set it to "false", a restart of FF always
> returns it to "true".

This is bug 286553


Comment 8

13 years ago
Forget my last comment! I had an error in my user.js file.

Comment 9

13 years ago
This caused bug 300864
aaron: there is no patch on this bug...

Comment 11

13 years ago
(In reply to comment #10)
> aaron: there is no patch on this bug...

Okay, wrong bug. Whichever bug was used to turn on XUL error pages caused the
problem.
(Assignee)

Comment 12

13 years ago
Created attachment 196144 [details] [diff] [review]
version 0.1

It says in adamlock@eircom.net's bugzilla name "not reading bugs". I'm guessing
that he still does reviews, so requesting review from him.

It seems docshells are pretty static regarding prefs, so when we add observing
for one, maybe we should observe the others as well? That'd be a new bug in any
case.
Assignee: adamlock → vhaarr+bmo
Status: NEW → ASSIGNED
Attachment #196144 - Flags: review?(adamlock)

Updated

13 years ago
Attachment #196144 - Flags: review?(adamlock) → review?(bzbarsky)
Comment on attachment 196144 [details] [diff] [review]
version 0.1

biesi, could you take a look at this?  I won't be able to for a good long
while....  If you don't have time either, just punt back to me, I guess.
Attachment #196144 - Flags: review?(bzbarsky) → review?(cbiesinger)
Comment on attachment 196144 [details] [diff] [review]
version 0.1

yes, I think docshell should observe the other prefs too.


+    if (!nsCRT::strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID) &&
+	 !nsCRT::strcmp(aData,
+	   NS_LITERAL_STRING("browser.xul.error_pages.enabled").get()) &&
+	 mObserveErrorPages) {

you should move the mObserveErrorPages check first, since it's fastest to check

r=biesi with that; thanks for the patch!
Attachment #196144 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 15

13 years ago
Created attachment 196598 [details] [diff] [review]
version 0.2

biesi: Thanks!

When we're done with this bug, I'll file a new one about removing the mPrefs
member from docshell altogether, and make use of pref observers instead.
Attachment #196144 - Attachment is obsolete: true
Attachment #196598 - Flags: superreview?(jst)
Attachment #196598 - Flags: review+
Comment on attachment 196598 [details] [diff] [review]
version 0.2

sr=jst
Attachment #196598 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 17

13 years ago
biesi: Think you could help me check this in?

Filed bug 309654.
Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.743; previous revision: 1.742
done
Checking in docshell/base/nsDocShell.h;
/cvsroot/mozilla/docshell/base/nsDocShell.h,v  <--  nsDocShell.h
new revision: 1.191; previous revision: 1.190
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Depends on: 906282
You need to log in before you can comment on or make changes to this bug.