Closed Bug 585565 Opened 14 years ago Closed 14 years ago

The alternate style view stylesheet filter should work

Categories

(DevTools :: General, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jwalker, Assigned: msucan)

References

Details

(Whiteboard: [strings])

Attachments

(1 file, 5 obsolete files)

Currently it’s only a dummy. It should filter the rules display to only show the selected sheet.
We could temporarily turn this off, and I don't think this is a blocker from shipping the feature.
Whiteboard: [kd4b5]
Blocks: 586984
Assignee: nobody → jwalker
Assignee: jwalker → mihai.sucan
We are turning this back on for beta 5 so we can get the string in for l10n, removal is possible if we can't make it.
Attached patch menupopup problem (obsolete) — Splinter Review
Submitting a minimal patch that applies cleanly ontop of the latest style panel patches from bug 582596.

This shows the blocker I bumped into.
Neil: We have again a problem with xul:browser inside xul:panel. We cannot use a <select> element inside the XHTML page of the new Style panel. When I click it, the select drop-down is not displayed.

I switched to a xul:menulist which works better, but the menulist label is not displayed. What can we do?

Should I switch a home-cooked drop-down? A <div> with a CSS :hover.

Thank you very much!
Status: NEW → ASSIGNED
Depends on: 590090
Attached patch proposed fix (obsolete) — Splinter Review
This is the proposed patch. It should apply cleanly on top of the latest style panel patches from bug 582596.

Please note:

1. As we discussed in yesterday's meeting, when UA sheets are enabled we are slower.

2. We need to check if UA rules match parent elements, just like in the proposed patch for bug 585577. That patch will need to be updated to take into consideration the UA rules as well.

(it's beyond the purpose of this patch, I believe)

3. We have a problem with CSS rule selector parsing, reported in bug 590090. This is very much obvious when you look into the list of rules generated in the property view of the Style panel. We see broken selectors.

4. I had to use a div with :hover to show a "home-made" drop-down, instead of a xul:menulist and instead of a pure HTML <select> element. We can't use any of the two, as mentioned in the previous comment (yay for xul:browser in xul:panel, hehe).


That's all for now. Looking forward to your feedback! Thank you!
Attachment #468656 - Flags: feedback?(jwalker)
I've added the i10n strings to the main patch, might take the UI bit too, and open a bug against the fact that it doesn't filter.
(In reply to comment #4)
> Neil: We have again a problem with xul:browser inside xul:panel. We cannot use
> a <select> element inside the XHTML page of the new Style panel. When I click
> it, the select drop-down is not displayed.

I'm guessing this is on Windows? I don't see an issue on Mac. Is the select dropdown opening, or just not responding to the mouse? Does the keyboard work?

> I switched to a xul:menulist which works better, but the menulist label is not
> displayed. What can we do?

If you're using a menulist in an xhtml file, you probably have extra whitespace around the items. If you put all of the menulist content on one line or just put the opening tag on the previous line such that there isn't any whitespace between the popup and each item, it should work.
 
> Should I switch a home-cooked drop-down? A <div> with a CSS :hover.

You should definitely not do this.
(In reply to comment #7)
> (In reply to comment #4)
> > Neil: We have again a problem with xul:browser inside xul:panel. We cannot use
> > a <select> element inside the XHTML page of the new Style panel. When I click
> > it, the select drop-down is not displayed.
> 
> I'm guessing this is on Windows? I don't see an issue on Mac. Is the select
> dropdown opening, or just not responding to the mouse? Does the keyboard work?

This is on Linux.

The problem: the drop-down of the <select> never shows, even if I click it, even if I press enter on it (when it's focused). Keyboard seems to otherwise work.

I believe it's a reincarnation of the same problem: tooltips don't show, context menus don't work, etc. :)

(yay for xul:browser in xul:panel, hehe)


> > I switched to a xul:menulist which works better, but the menulist label is not
> > displayed. What can we do?
> 
> If you're using a menulist in an xhtml file, you probably have extra whitespace
> around the items. If you put all of the menulist content on one line or just
> put the opening tag on the previous line such that there isn't any whitespace
> between the popup and each item, it should work.

Uh oh, I am sure I had whitespaces. Will try without whitespaces to see if it works.

> > Should I switch a home-cooked drop-down? A <div> with a CSS :hover.
> 
> You should definitely not do this.

I did this, because we need to land the new strings in beta 5. I hope I can get the <select> working if you have tips on how to do it, or at least the xul:menulist.

Thank you very much for your answers!
Attached patch updated fix (obsolete) — Splinter Review
This is the updated patch. Changes:

- rebased on top of the latest style panel patches (bug 582596).
- strings are no longer included in this patch, they are in the main patches already.
- implemented a simple rule.selectorText parser that no longer fails parsing as described in bug 590090.
- switched to a xul:menulist native element, which looks and behaves much better than the "home-cooked" one from the previous patch. Big thanks to Neil Deakin!
- fixed a bug in _findUARules() which caused the Style panel to show duplicated rules.

Feedback for improvements is always welcome! Thanks!
Attachment #468397 - Attachment is obsolete: true
Attachment #468656 - Attachment is obsolete: true
Attachment #469563 - Flags: feedback?(jwalker)
Attachment #468656 - Flags: feedback?(jwalker)
Whiteboard: [kd4b5] → [kd4b6]
Whiteboard: [kd4b6] → [kd4b6] [strings]
Attached patch rebased patch (obsolete) — Splinter Review
Updated patch. Changes:

- rebased patch on top of the latest style panel patches from bug 582596.
- moved the strings back into this patch as requested by Axel.
Attachment #469563 - Attachment is obsolete: true
Attachment #470498 - Flags: feedback?(jwalker)
Attachment #469563 - Flags: feedback?(jwalker)
Blocks: 590090
No longer depends on: 590090
Attachment #470498 - Flags: feedback?(jwalker) → feedback+
Comment on attachment 470498 [details] [diff] [review]
rebased patch

Asking for review from Dolske. This is an important bit of functionality for the new Style panel that we want in Firefox 4. It is also important for beta6, because the patch includes strings.
Attachment #470498 - Flags: review?(dolske)
Blocks: 589849
Blocks: 593107
Attached patch upload 3 (rebase) (obsolete) — Splinter Review
rebase
Attachment #470498 - Attachment is obsolete: true
Attachment #471845 - Flags: review?(dolske)
Attachment #470498 - Flags: review?(dolske)
Attached patch upload 6Splinter Review
merge fix
Attachment #471845 - Attachment is obsolete: true
Attachment #471845 - Flags: review?(dolske)
Removing items from kd4b6.
Whiteboard: [kd4b6] [strings] → [strings]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: