'.no-such-class > text' will match any <text> element inside a <stack> in XBL binding

RESOLVED WORKSFORME

Status

()

Core
XUL
P3
normal
RESOLVED WORKSFORME
18 years ago
16 years ago

People

(Reporter: Sean Richardson, Assigned: jag (Peter Annema))

Tracking

({polish})

Trunk
polish
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Hixie-P3][need info])

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
This appears to be no more than a minor polish issue, but as it is the 
progress meter is extending 2px below the status bar, causing a band of
white 2px tall to appear between the status bar and the taskbar. Or, if
that white band is meant to be there, the progress meter's area is spilling
over into it. It just plain looks oversize. 

The white band itself just looks silly, even if the progress meter stopped
stomping on it. If it is meant to impart a 3-d appearance, it should probably
be two colours, each in a 1px band.

It appears that the progress meter is being positioned 1px to the right and
2px below the region reserved for it. This can be most easily seen on a slower
computer while a new window is loading; the area where the progress meter will
appear is initially transparent, showing how its region starts below the top of 
the status bar and extends into the white band.

Screenshot zoomed to 200% to follow in attachment.

Sorry if this is a dup; searched for "barber" and "progress meter" in summary
before filing.
(Reporter)

Comment 1

18 years ago
Created attachment 8811 [details]
200% zoom of progress meter showing its position
(Reporter)

Updated

18 years ago
Keywords: polish

Comment 2

18 years ago
Build 2000051808. I see exactly the same on Linux as well.

Comment 3

18 years ago
This looks like a dup of bug 25812, but I can't be sure.
OS: Windows NT → All
Hardware: PC → All

Comment 4

18 years ago
Reproduced on Mac build 2000051808.. Since this is happening both in the browser 
window (this bug) and in the download window (bug 25812), I'd guess it's 
something wrong with the progressmeter widget.
Assignee: bdonohoe → trudelle
Component: User Interface: Design Feedback → XP Toolkit/Widgets
QA Contact: mpt → jrgm
(Reporter)

Comment 5

18 years ago
No, bug 25812, "Download status appearance errors - progress bar" reports a
different problem: the actively drawn portion of the "thermometer" being
drawn beyond the bottom of the background of the thermometer. It is also
not a problem in current builds: the progress window has been overhauled.

Here, the entire progress meter, regardless of its state, extends below the
bottom of the status bar at all times.

Comment 6

18 years ago
There are two things that are causing this little bit of overflow;
one is just a bit of stylistic polish, but the other seems to be 
a serious error in CSS selectors.

The first, polish issue is that 'max-height: 1.5em' is being set
on the progressmeter. This should really be set on the only bit
of the content that has intrinsic size: the '.progress-text' <text>
element in the interior of the <progressmeter> XBL. (Otherwise, if
the contents of the this '.progress-text' are greater than 1.5em, 
you get this inner content spilling out of the progressmeter.

The second issue is that somehow a child selector beginning in a 
class that is not used anywhere, and ending in the element 'text', 
is matching the inner <text> element of the <progressmeter> (and
providing the style that 'spills out' of the <progressmeter>).

This is the selector (adapted from one in 'sidebarOverlay.css' that
was the cause in the overall navigator.xul).

   .no-class-by-this-name > text {
       margin: 4px;
       font-family: sans-serif;
   }

Here's some .xul and .css to demonstrate the problem:

@namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"); 
@namespace html url("http://www.w3.org/TR/REC-html40"); 

progressmeter 
  {
    border              : 2px solid red;
   /* This max-height should not be set here; 
      it should be in .progress-text (?) */
    max-height          : 1.5em; 
  }
.progress-text 
  {
   /* The max-height should be set here, not in progressmeter (?) */
   /*  max-height          : 1.5em; */
  }

/* XXX how the heck is this selector matching
   the inner <text> of the progressmeter */
.no-class-by-this-name > text {
    margin: 4px;
    font-family: sans-serif;
}

progressmeter > .progressmeter-internal-box 
  {
    border              : 2px solid #FFCC33; /* gold */
  }
progressmeter > .progressmeter-internal-box > .progressmeter-stack
  {
    border              : 2px solid green;
  }


<?xml version="1.0"?>

<?xml-stylesheet href="progmeter.css" type="text/css"?>

<window 
  id="main-window" 
  xmlns:html="http://www.w3.org/TR/REC-html40"
  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
  height="300" width="300" orient="vertical"> 

      <progressmeter progresstext="Is this sans-serif?"/> 
      
      <box><html style="margin-top:20px;">
        Notice that this text is serif!
      </html></box> 

</window>


I'll attach a .zip file with these files in a moment.  Place the .xul
and .css into the same directory as navigator.xul and then run:

  .\netscp6 -chrome chrome://navigator/content/foo.xul

Assignee: trudelle → ben
Severity: minor → normal

Comment 7

18 years ago
Created attachment 8955 [details]
foo.xul, progmeter.css in a .zip file

Comment 8

18 years ago
I think this may only affect <xul:stack> (or, it may only be apparent in 
xul:stack, but the match may be nullified by something more specific in the 
cascade for other xbl bindings).

Comment 9

18 years ago
Added "patch" keyword.

Ben, can we take the attached patch?
Keywords: patch
Target Milestone: --- → M17

Comment 10

18 years ago
Uh, well that's not a patch, it's just to demonstrate the problem clearly.

I'm going to file a separate bug to cover the point that the 'max-height:'
should be set on .progress-text and not directly on the progressmeter, and
Ben can deal with that issue there.

This bug can then just cover the serious problem that CSS selectors of the 
form '.some-nonexistent-class > text' will match *any* <text> element 
contained in certain XBL bindings (but perhaps only those that are inside a 
stack, though).
Keywords: patch

Updated

18 years ago
Summary: Progress meter (barber pole) extends below status bar → '.no-such-class > text' will match any <text> element inside a <stack> in XBL binding
*** Bug 36201 has been marked as a duplicate of this bug. ***
this is a dupe of 36201, but since this one has more "activity", will break the
conventions and mark 36201 a dupe of this
dave, who should get this? 
Assignee: ben → hyatt

Comment 14

18 years ago
Fixed.  Style tree is now kept in sync with frame tree for XBL.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 15

18 years ago
reopening. This still happens. Put this rule in 'xul.css', start the browser
and look at the progressmeter -- the text will be big and green.

.ieosofasdkljiasiudfuoioipiuqowier > text {
    font-family: monospace;
    font-size: 36px;
    color: green;
}

That selector should match nothing, of course. Perhaps this is just a 
curiosity, but perhaps this is causing a lot of load in the style system,
since this is effectively operating as '* > text'


Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 16

18 years ago
need info: is this causing [perf] problems, or is it benign?  nsbeta3 or future?
Whiteboard: [need info]

Comment 17

18 years ago
-> Future 
Target Milestone: M17 → Future
Whiteboard: [need info] → [Hixie-P3][need info]

Comment 18

16 years ago
--> default owner
Assignee: hyatt → jaggernaut
Status: REOPENED → NEW
Target Milestone: Future → ---

Comment 19

16 years ago
whatever, or que sera, sera.
Status: NEW → RESOLVED
Last Resolved: 18 years ago16 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.