Closed
Bug 692640
Opened 13 years ago
Closed 13 years ago
Video statistics text overlaps itself on small dimension videos
Categories
(Toolkit :: Video/Audio Controls, defect)
Toolkit
Video/Audio Controls
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: cpearce, Assigned: jaws)
Details
Attachments
(3 files, 1 obsolete file)
96.24 KB,
image/png
|
Details | |
10.12 KB,
patch
|
Dolske
:
review+
|
Details | Diff | Splinter Review |
10.11 KB,
patch
|
Details | Diff | Splinter Review |
On small dimension videos such as http://pearce.org.nz/video/320x240.ogg the text on the video statistics overlaps itself (see attached screen shot). This doesn't seem to happen on larger dimensioned videos such as http://pearce.org.nz/video/arctic_giant.ogg
Assignee | ||
Comment 1•13 years ago
|
||
What should be the expected outcome here? If the statistics overflow the element then portions of the text may not be drawn. If we truncate the text then it may become useless. Any ideas/feedback here is greatly appreciated.
Assignee: nobody → jwein
Status: NEW → ASSIGNED
Assignee | ||
Updated•13 years ago
|
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee | ||
Comment 2•13 years ago
|
||
This patch makes the Frames parsed/decoded/presented/painted stats span two columns for both the label and value. Still to figure out is a solution for the other statistics fields. This change also means that large videos will have frame statistic values visually far from the frame statistic labels.
Reporter | ||
Comment 3•13 years ago
|
||
Expected outcome would be text being more readable. ;) I'm not sure what the best solution is here. Maybe move the volume, channels and sample rate up (or down) to allow room for the readyState and networkState text? Or use CSS3 columns so that the number of columns naturally shrinks when there's not enough horizontal space? You can always run out of vertical space eventually no matter what you do I guess.
Assignee | ||
Comment 4•13 years ago
|
||
This patch moves all statistics to a single column and allows the statistics to be rendered outside of the video element if the video element is not large enough to contain them. The absolute positioning of the table is used to keep the video control bar within the video element. If the video control bar is moved outside of the content area then it will be inaccessible.
Attachment #565386 -
Attachment is obsolete: true
Attachment #566336 -
Flags: review?(dolske)
Comment 5•13 years ago
|
||
Comment on attachment 566336 [details] [diff] [review] Patch for bug 692640 Review of attachment 566336 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/themes/pinstripe/global/media/videocontrols.css @@ +181,5 @@ > background: url(chrome://global/skin/media/error.png) no-repeat center; > } > > /* Statistics formatting */ > +html|*.statsDiv { html|.statsDiv? @@ +188,3 @@ > html|td { > + height: 15px; > + max-height: 15px; Where does "15px" come from? Perhaps you want "1em" here?
Attachment #566336 -
Flags: review?(dolske) → review+
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to Justin Dolske [:Dolske] from comment #5) > Comment on attachment 566336 [details] [diff] [review] [diff] [details] [review] > Patch for bug 692640 > > Review of attachment 566336 [details] [diff] [review] [diff] [details] [review]: > ----------------------------------------------------------------- > > ::: toolkit/themes/pinstripe/global/media/videocontrols.css > @@ +181,5 @@ > > background: url(chrome://global/skin/media/error.png) no-repeat center; > > } > > > > /* Statistics formatting */ > > +html|*.statsDiv { > > html|.statsDiv? According to the grammar specified by section 3.1 here, the element name is required when using the namespace selector. I've used the asterisk here to remove the need to also check the specific element name. > @@ +188,3 @@ > > html|td { > > + height: 15px; > > + max-height: 15px; > > Where does "15px" come from? Perhaps you want "1em" here? 15px came from a font-size of 11px and 2px of padding on the top and bottom. I've switched it to 1em for simplicity and readability.
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to Jared Wein [:jwein] from comment #6) > > ::: toolkit/themes/pinstripe/global/media/videocontrols.css > > @@ +181,5 @@ > > > background: url(chrome://global/skin/media/error.png) no-repeat center; > > > } > > > > > > /* Statistics formatting */ > > > +html|*.statsDiv { > > > > html|.statsDiv? > > According to the grammar specified by section 3.1 here, the element name is > required when using the namespace selector. I've used the asterisk here to > remove the need to also check the specific element name. > Sorry, I meant to include a link to my reference: http://www.w3.org/1999/06/25/WD-css3-namespace-19990625/#tag-selector
Assignee | ||
Comment 8•13 years ago
|
||
Assignee | ||
Comment 9•13 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/372c8e0fedcc
Whiteboard: [fixed-in-fx-team]
Comment 10•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/372c8e0fedcc
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla10
You need to log in
before you can comment on or make changes to this bug.
Description
•