687202 - Improve <meter> a11y by adding ticks showing 'low' and 'high' values

Status: NEW

(Core :: Layout: Form Controls, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

To improve the visual a11y of the element, I have a patch that adds two ticks, one representing the low mark and one representing the high mark. If low==high, only one is draw and if low==min or, the low mark isn't draw. Same thing for high==max.
The idea is with low and high marks, the user will be able to see the limits. Where the value has to be might be understandable from the contexts: if the meter represents the number of bugs in Firefox, it will have to be low, if it represents the number of Firefox users, it will have to be high.

This patch should be nearly ready. I might have to do a bit of clean-up. Nothing important.

David, how does it sound?

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 560663 [details] [diff] [review]
Patch v1

Review of attachment 560663 [details] [diff] [review]:
-----------------------------------------------------------------

Seems to me that this could paint outside the content-box or even the border-box of the meter. In that case, your nsMeterFrame::Reflow needs to add the relevant overflow area before calling FinishAndStoreOverflow --- definitely to the visual overflow area, and possibly to the scrollable overflow area, if you want to be able to scroll to see these marks. Your display item's GetBounds needs to include the overflow area too (possibly just by returning the frame's overflow area).

::: layout/forms/nsMeterFrame.cpp
@@ +302,5 @@
> +class LimitsPainter {
> +public:
> +  static void
> +  PaintLimit(nsRenderingContext* aCtx, const nsRect& aRect, float aValue,
> +             bool aIsHorizontal, bool aIsRTL)

probably a little cleaner to pass the orientation and direction directly and test for values in the body of this method. boolean parameters aren't good.

@@ +307,5 @@
> +  {
> +    // Points come from the coordinates on a 5X4 unit box.
> +    static const PRInt32 arrowPolygonX[] = { -2,  2, 0 };
> +    static const PRInt32 arrowPolygonY[] = { -4, -4, 0 };
> +    static const PRInt32 arrowNumPoints = sizeof(arrowPolygonX) / sizeof(PRInt32);

use NS_ARRAY_LENGTH

@@ +330,5 @@
> +      } else if (!aIsRTL) {
> +        paintPolygon[i] = nsPoint(x + nsPresContext::CSSPixelsToAppUnits(arrowPolygonY[i]),
> +                                  y + nsPresContext::CSSPixelsToAppUnits(arrowPolygonX[i]));
> +      } else {
> +        paintPolygon[i] = nsPoint(x + nsPresContext::CSSPixelsToAppUnits(abs(arrowPolygonY[i])),

Why not x - nsPresContext::CSSPixelsToAppUnits(arrowPolygonY[i])?

@@ +341,5 @@
> +  static void
> +  Paint(nsIFrame* aFrame,
> +        nsRenderingContext* aCtx,
> +        const nsRect& aDirtyRect,
> +        nsPoint aPt)

Probably simpler just to make Paint() and PaintLimit() static global functions.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.