Closed Bug 213994 Opened 19 years ago Closed 19 years ago

Hide Incompatible Themes

Categories

(Firefox :: Preferences, defect)

x86
All
defect
Not set
major

Tracking

()

VERIFIED FIXED
Firebird0.7

People

(Reporter: mozilla-bugs, Assigned: mconnor)

References

Details

Attachments

(1 file, 5 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030726 Mozilla Firebird/0.6.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030726 Mozilla Firebird/0.6.1

Hello.

First of all, I'm using a fresh ~/.phoenix profile directory.  I've downloaded
tonight's nightly and extracted it.  I customized it in options and then chose
to install a few new themes from the default web site.  However, two problems
followed.

(1) When 'Use this theme' checkbox is checked the browser *did not* apply the theme.
(2) When I manually selected a new theme in 'Options' and clicked OK the
dialogue box refused to vanish and the theme was not applied.  However, when I
clicked on the default theme and clicked OK the dialogue box disappeared and the
theme remained on default.

Also, the texturizer theme site popped up a particular dialogue box repeatedly
which said '421 There are too many connections from your internet address' even
though I only went to it once from my IP address.

I'd be grateful if firebird or the themes could be updated to work with one another.

With regards.
Dhruba Bandopadhyay.

Reproducible: Always

Steps to Reproduce:
1. Open tonight's firebird nightly with new profile directory.
2. Install a new theme.
3. Try and apply it.

Actual Results:  
The dialogue box refused to disappear and theme was not applied.  Any new themes
could not be applied at all.

Expected Results:  
It should :-

(1) Apply the theme immediately when the checkbox 'Use this theme' is checked
when installing a new theme.
(2) Apply the theme when asked to do so in 'Options'.
I got this 421 to many internet connections error on the texturiser themes page
earlier this evening when I was using the Firebird 6.1 release candidate but
Iwas able to install themes successfully in that build
As posted on MozillaZine, this is due to the skinVersion change.  

Therefore this bug is probably invalid.  

But it does raise another issue:  how to prevent the installation of
incompatible themes in the first place.  It certainly sounds like the installer
says it was succesful.  
See if you can install and use the Pinball 0.6.7 theme from
http://members.rogers.com/dpjames/firebird/pinball.html

If you can, then this bug is invalid since I updated Pinball to use version
skinVersion 1.5
*** Bug 214005 has been marked as a duplicate of this bug. ***
The pinball theme applies fine but no other ones do.  However, user should be
alerted of incorrect theme versions or unsuccessful installations somehow as
pointed out.  Also, how long before all other texturizer themes get updated? 
Many thanks.
Bug is definitely INVALID, older themes are not going to be supported due to
changes in CSS support for -moz attributes.  Its just been a long while since
the skinVersion changed afaik.
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
Mike:

Alan and Dhruba are correct though that Firebird reported as successful the
installation of incompatible/deprecated themes, and that is definitely a bug.
The fact that the reporter originally misdiagnosed the bug doesn't mean it isn't
one (I'm eating my own words here I realize...). Anyway, changing the summary
from "Cannot apply newly installed themes" to "Firebird "successfully" installs
incompatible themes".
Status: RESOLVED → UNCONFIRMED
OS: Linux → All
Resolution: INVALID → ---
Summary: Cannot apply newly installed themes → Firebird "successfully" installs incompatible themes
Confirming and taking QA contact.
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: asa → davidpjames
taking
Assignee: blake → mconnor
Status: NEW → ASSIGNED
Attached patch patch (obsolete) — Splinter Review
since the description info about this being an incompatible theme isn't
visible, we're running into this now, patch restores the popup functionality
Comment on attachment 129107 [details] [diff] [review]
patch

need r= and checkin :)
Attachment #129107 - Flags: review?(noririty)
*** Bug 214929 has been marked as a duplicate of this bug. ***
Comment on attachment 129107 [details] [diff] [review]
patch

We can't close dialogbox after pressing "OK".
Attachment #129107 - Flags: review?(noririty) → review-
Right, we need to pick another theme since the one we picked wasn't valid.

We can bypass this, but I don't think its necessarily the right way of doing this.
patch checked in
Status: ASSIGNED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Verified using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b)
Gecko/20030817 Mozilla Firebird/0.6.1+
Status: RESOLVED → VERIFIED
BZZZZT

Wrong fix, and you busted the options dialog. Showing an alert after the fact
(and after the user may have switched away from themes and to somewhere else) is
not good enough UI to indicate that their theme choice is invalid. Either the
theme should not show up in the list or it should not be selectable or something. 

Whatever the case, this patch broke the options dialog because it attempted to
access nodes in the dialog that are not present if the user switches panels away
from Options. You cannot access the pref pane document from Options callback
functions. Please, please, please, be more careful. 
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Ben, I wasn't even thinking of applyTheme being a callback, I should have known
better.  As it was I was just fixing broken code by providing correct
variables, hence I wasn't thinking it was broken before.

This patch simplifies the dialog, however to avoid this being a problem with
UI, the theme install needs a check for validity.  Hopefully once the theme
versions settle down, the theme site's browser detection work will allow
effective limiting of choices, lessening the chance of installing an
incompatible theme.

We also need to fix bug 214465 and bug 191992 for this fix to be ideal.
Attachment #129107 - Attachment is obsolete: true
Depends on: 214465
Attached patch ugly hack to show default theme (obsolete) — Splinter Review
also removes the extra var oldTheme = null which isn't needed.
Attached patch another hack while I'm at it (obsolete) — Splinter Review
this one is to hide the modern theme as well
Attachment #130699 - Attachment is obsolete: true
Attachment #130740 - Attachment is obsolete: true
Comment on attachment 130741 [details] [diff] [review]
another hack while I'm at it

Ben, I know this has a couple of ugly hacks going, we can strip those out once
we figure out what the Marvels of RDF are causing.  (bug 191992 and bug 214465
to be precise)
Attachment #130741 - Flags: review?(bugs)
*** Bug 215707 has been marked as a duplicate of this bug. ***
On further investigation, I don't know if this is what broke prefs after all,
since backing this out fixed nothing as far as I can tell.

In any case, I think the last patch I submitted is a better solution, and fixes
a couple other bugs in the process (via crappy hacks, but it works)
Component: General → Preferences
Blocks: 215707
Attachment #130741 - Flags: review?(bugs)
Attached patch fix stupid perf bug (obsolete) — Splinter Review
bleh
Attachment #130741 - Attachment is obsolete: true
Comment on attachment 131150 [details] [diff] [review]
fix stupid perf bug

fixes stupid perf bug from repeating removeInvalidThemes() call for each theme
(genious move there)

now that bug 214465 is fixed, I'd like to see this in for 0.7 if possible,
should be low-risk, I've put plenty of testing into it by now.
Attachment #131150 - Flags: review?(noririty)
Status: REOPENED → ASSIGNED
-> 0.7, in the hopes that we can take this fix instead of fixing just 215707
Summary: Firebird "successfully" installs incompatible themes → Hide Incompatible Themes
Target Milestone: --- → Firebird0.7
blake wanted the modern entry left in the list (bug 191992 covers this)
Attachment #131150 - Attachment is obsolete: true
Attachment #131306 - Flags: review+
patch landed on trunk, waiting on ok for landing on branch for 0.7
Fixed on trunk, still need to get this on the branch for 1.5/0.7

Bryner, okay with taking this for 0.7?  Its in the current nightly, will not
cause problems, and fixed a regression caused by the improper backout of the
previous patch for this bug.

Already have a blanket a=asa for checkins to the branch that are /browser or
/toolkit specific, just need a module peer/owner to okay it as well.
Attachment #131150 - Flags: review?(noririty)
I think this can go into 0.7.
Fixed in 0.7.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
verified fixed 2003-11-09
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.