253332 - [AltSS] Implement Page Style > None

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[fantasai]

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

Comment 1

[fantasai]

Attachment: patch

Comment 2

[fantasai]

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

[[e-mail]]

Not me; FireFox won't need any xpfe changes.

Comment 4

[fantasai]

Attachment: patch

Comment 5

[fantasai]

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.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 8

[fantasai]

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...

Comment 9

[fantasai]

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.

Comment 10

[fantasai]

Attachment: patch with "none" as "Basic Page Style" (instead of "Default Style")

Comment 11

[fantasai]

Attachment: patch without menu changes

Comment 12

[fantasai]

Attachment: patch (holding out against bitrot here...)

Comment 13

[Steffen Wilberg]

Comment on attachment 155451 [details] [diff] [review]
patch

Removing requests from obsolete patch.

Comment 14

[fantasai]

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.

Comment 15

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 156965 [details] [diff] [review]
patch (holding out against bitrot here...)

OK. sr=[e-mail]

Comment 16

[Doug Wright]

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?

Comment 17

[fantasai]

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.

Comment 18

[Bill Gianopoulos [:WG9s]]

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.