Closed
Bug 253332
Opened 20 years ago
Closed 20 years ago
[AltSS] Implement Page Style > None
Categories
(Firefox :: Menus, defect)
Tracking
()
RESOLVED
FIXED
Firefox1.0beta
People
(Reporter: fantasai.bugs, Assigned: fantasai.bugs)
References
Details
(Keywords: fixed-aviary1.0, Whiteboard: [have patch] affects l10n, Gecko)
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
dbaron
:
review+
bugs
:
superreview+
|
Details | Diff | Splinter Review |
1. Backport author style level disabling backend from bug 32372 to 1.0 branch 2. Add "Use Style" menu to View menu - makes AltSS switcher keyboard-accessible (fixes bug 216424) - makes it possible to use "none" option when page does not define an alternate style sheet (fixes bug 224250) 3. Make "none" option hook into style disabling backend so that it a) disables ALL author style sheets (& style attr) b) disables HTML presentational attributes in preshint level of cascade
The patch has two problems right now: nsIDOMNSDocumentStyle.idl is using the wrong license and the patch to browser.dtd changes a couple lines it shouldn't touch. I'll attach a new one in the next day or so, but I'm CCing you (Neil and dbaron) since I plan to ask you for review after that. (If I should be asking someone else, let me know, eh?)
Comment 3•20 years ago
|
||
Not me; FireFox won't need any xpfe changes.
Attachment #155408 -
Attachment is obsolete: true
Comment on attachment 155451 [details] [diff] [review] patch bug 32372, which is the equivalent bug for the trunk (minus the Firefox UI part), has been fixed, reviewed, and checked-in already. No one has complained to me about it yet. The changes have been tested against a couple alternate-style-sheet-enabled pages I have lying around, one with and one without a preferred style. If I've asked the wrong people for review for this patch, please correct me.
Attachment #155451 -
Flags: superreview?(bugs)
Attachment #155451 -
Flags: review?(dbaron)
Attachment #155451 -
Flags: approval-aviary?
Comment 6•20 years ago
|
||
Comment on attachment 155451 [details] [diff] [review] patch >+nsDocument::GetPreferredStylesheetSet(nsAString& aStyleTitle) >+{ >+ GetHeaderData(nsHTMLAtoms::headerDefaultStyle, aStyleTitle); >+ return NS_OK; >+} Why this rather than what's on the trunk?
Comment 7•20 years ago
|
||
My previous comment applies to the missing css loader changes as well. Also, why the navigatorOverlay.xul differences (missing type="radio" and different line breaking (when applying patches to branches, only add differences where they're necessary -- it makes later merging easier))? Also, don't forget to land the firefox changes (the changes in mozilla/browser) on the trunk too. You'll probably need to discuss the browser-menubar.inc changes with Ben, but I'm glad to see the UI somewhere that's keyboard-accessible.
Updated•20 years ago
|
Attachment #155451 -
Flags: review?(dbaron) → review+
wrt GetHeaderData, that's because of bug 254445 wrt type="radio", that's because <menupopup> afaict doesn't really have a type attribute, so I need to remove that from the trunk as well. wrt merging, is it ok if I have the navigatorOverlay.xul changes on the branch and then make the trunk match the changes? wrt browser-menubar.inc, I've asked Ben for sr. Dunno whether it'll happen by tonight, though...
TRANSCRIPT BenG: OK, I'm ok with the patch in general but I think you should hold off on the menu items, or talk to blake about them since he's wearing the menu nazi hat at the moment. fantasai: ok, but if that thing's going to be keyboard-accessible it *has* to be in the Menus. BenG: He can take over the review from here, but all the back end stuff seems fine if dbaron is happy with it --- Note: This patch will fix all dependencies as well.
Summary: Implement Use Style > None → [AltSS] Implement Page Style > None
Attachment #155451 -
Flags: superreview?(bugs) → superreview?(firefox)
Assignee | ||
Comment 10•20 years ago
|
||
Assignee | ||
Comment 11•20 years ago
|
||
Attachment #155451 -
Attachment is obsolete: true
Attachment #156299 -
Attachment is obsolete: true
Assignee | ||
Comment 12•20 years ago
|
||
Attachment #156647 -
Attachment is obsolete: true
Updated•20 years ago
|
Attachment #156965 -
Flags: review+
Comment 13•20 years ago
|
||
Comment on attachment 155451 [details] [diff] [review] patch Removing requests from obsolete patch.
Attachment #155451 -
Flags: superreview?(firefox)
Attachment #155451 -
Flags: approval-aviary?
Updated•20 years ago
|
Attachment #156965 -
Flags: superreview?(firefox)
Attachment #156965 -
Flags: approval-aviary?
Updated•20 years ago
|
Flags: blocking-aviary1.0PR?
Whiteboard: [have patch] affects l10n - need review blake
Attachment #156965 -
Flags: superreview?(firefox)
Attachment #156965 -
Flags: approval-aviary?
Assignee | ||
Comment 14•20 years ago
|
||
wrt approval-aviary, see comment 9 wrt sr by blake, the menu changes have been removed from the original patch at his request The patch is just waiting for check in.
Whiteboard: [have patch] affects l10n - need review blake → [have patch] affects l10n, Gecko
Comment 15•20 years ago
|
||
Comment on attachment 156965 [details] [diff] [review] patch (holding out against bitrot here...) OK. sr=ben@mozilla.org
Attachment #156965 -
Flags: superreview+
Comment 16•20 years ago
|
||
Sorry if this is considered 'advocacy', but why is this in the aviary branch, when support for alternate stylesheets has just been removed? Or are we getting AltSS support back now? How do we use this new functionality?
Assignee | ||
Comment 17•20 years ago
|
||
Fixed by vladimir's checkin yesterday (2004-08-26)
> How do we use this new functionality?
Add
#define SHOW_ALT_SS_UI 1
to the top of mozilla/browser/base/content/browser.xul and browser.js
and rebuild. Blake didn't remove the code, he just ifdef-ed it out.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 18•20 years ago
|
||
It is still not compliant with the standards because there is still no way to select Page Style =-> None if the only style sheet on the page has no title, because even if you turn all the removed code back on the icon does NOT display in this case.
Keywords: fixed-aviary1.0
Updated•20 years ago
|
Flags: blocking-aviary1.0PR?
Updated•18 years ago
|
QA Contact: bugzilla → menus
You need to log in
before you can comment on or make changes to this bug.
Description
•