Closed Bug 1311857 Opened 8 years ago Closed 7 years ago

<input type="time"> inner components overflow (quite a bit) & stomp on later content, if it's given a small width

Categories

(Core :: Layout: Form Controls, defect, P2)

Product:
Core
Component:
Layout: Form Controls
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox52 --- wontfix
firefox55 --- fixed

People

(Reporter: dholbert, Assigned: jessica)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

testcase 1
668 bytes, text/html
Details
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
content not clipped on bottom edge of the content
8.02 KB, image/png
Details
(followup) Add bug number for expected test failure in datetime reftest.list
1.17 KB, patch
jessica
: review+
Details | Diff | Splinter Review
Attached file testcase 1 — 
STR:
 0. Set about:config pref dom.forms.datetime = true
 1. Load attached testcase.

EXPECTED RESULTS:
* "x" button should probably be at right edge of control.
* Internal controls should all be on a single line.
* When space isn't available, controls/content should probably be clipped at <input> borders. (just like text is clipped inside of a text <input>)
* Internal controls should definitely not visually overflow off of the <input> tag. (or not far, at least)

ACTUAL RESULTS:
* "x" button seems to have a specific desired horizontal position, regardless of control size.
* When space isn't available, internal controls wrap to multiple lines, and overflow quite far from the edge of the <input> (stomping on content that comes afterwards)

See the small plain <input> textfield at the very end of the testcase for a reference of how this probably should behave. (That <input> is too small for its contents, and the contents are just clipped.)
Summary: <input type="time"> needs to render nicely (clipped maybe?) if it's styled with a small width → <input type="time"> inner components overflow (quite a bit) & stomp on later content, if it's given a small width
Let's get the flexbox properties right so that we can use it as leverage for RTL.
Assignee: nobody → jjong
Mass wontfix for bugs affecting firefox 52.
status-firefox52: affected → wontfix
Attached image button_overflow.png (obsolete) — 
I've uploaded my current patch, but I still need some help. The patch fixes most of the cases, but there is still one problem with the case "Time, small height/width": the button is not clipped vertically. Note that you need to focus on the field and enter a value, you can do that by pressing the up/down key.

The implementation has changed quite a bit since this bug was reported. For example, the inner components are span instead of input texts, and the clear button appears only when there is some value in the input box.
(In reply to Jessica Jong [:jessica] from comment #4)
> Created attachment 8860308 [details]
> button_overflow.png
> 
> I've uploaded my current patch, but I still need some help. The patch fixes
> most of the cases, but there is still one problem with the case "Time, small
> height/width": the button is not clipped vertically. Note that you need to
> focus on the field and enter a value, you can do that by pressing the
> up/down key.

This issue only appears on Mac. On Ubuntu, it's clipped nicely.
Priority: -- → P2
Hi Daniel, with this patch, the test page (attachment 8803141 [details]) looks okay now. Could you help take a look? Thanks.
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review148884

This needs some reftests to prove that the issues shown in the attached testcase are actually fixed, & to help catch regressions in this behavior.

It may be tricky to reftest the exact rendering, but here are a few tricks that come to mind:

(1) You can set "color: white" to make the --:-- parts invisible, and then just detect overflowing content using the "outline" CSS property (which encapsulates all painting, including white-on-white) -- something like what's shown in this testcase:
https://jsfiddle.net/3379nzva/
With that trick, you could compare a skinny <input type="time"> against a skinny <input>, and they should match.

(For me, in Nightly, this ^^ testcase draws an extra large dotted rectangle around the input type="time" element.)

With that, you should be able to add a reftest along the lines of the "variety of widths", "variety of heights", and "small height/width" scenarios from the original testcase here.  (Where the parts with small heights and/or widths will fail without your patch, but pass with your patch)

(2) If you want to test the actual rendering of the contens -- e.g. to maybe test that "--:--" is left-aligned by default, even when the <input> is too small to fit everything, you could have this in your testcase:
 <input type="time" style="width: 50px">
...and this in your reference case:
 <input type="time" style="width: 200px">
...and have a sufficiently-large abspos div with "position:absolute;top:0;left: 40px;background:gray" or something like that, to cover up the right chunk of each input.

You might want to do something similar to test that the "x" icon is always right-aligned (when it shows up), even if the size is small, too.  (At least, it seems like that's how you're intending for things to work from testing the attached patch).  To test that, you'd want to cover up the *left* chunk of each input with a mask div, and style the <input> with "float:right" to position its right edge at a predictable location regardless of its width.

You might find some other useful reftesting tricks in the (similar) testcases in layout/reftests/forms/input/number, too.

::: layout/style/res/forms.css:1192
(Diff revision 2)
> +  overflow: hidden;
> +  white-space: nowrap;

Right now, you're setting 4 different components to each be "overflow: hidden".

This one here (for "input") makes sense -- at the edge of the control, where we care about clipping & overflow.  But why are the others necessary?  (Are they necessary?)

(I can imagine the text-content wrapper might need to be overflow:hidden as well, so that it doesn't overlap the "x" icon when space is limited.  Though: I'm not 100% sure the "x" icon is the most important thing to show in that scenario. Perhaps it'd be better to let the x icon get pushed out of view? *shrug*)
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review148910

Two specific notes on "overflow:hidden" usages that I'll bet you don't really need:

::: toolkit/content/widgets/datetimebox.css:16
(Diff revision 2)
> +  flex: 1;
>    cursor: default;
>    background-color: inherit;
>    color: inherit;
>    font-family: monospace;
> +  overflow: hidden;

Perhaps "min-width:0" works just as well here?

(I'll bet you really just want to allow this component to compress to be arbitrarily small -- and for a flex item, "min-width:0" is the simplest way to do that. "overflow:hidden" works too but it's a much larger hammer, because it adds a layer of scrollframe & other complexity.)

::: toolkit/content/widgets/datetimebox.css:22
(Diff revision 2)
> +}
> +
> +.datetime-input-edit-wrapper {
> +  flex: 1;
> +  justify-content: flex-start;
> +  overflow: hidden;

(I suspect this is the component that wraps the --:-- -- text. Do you really want this to be "overflow:hidden" and forcibly clip its text contents at the spot where the "x" icon would appear?  Or is "min-width:0" [to allow arbitrary shrinking] good enough?  I suspect "min-width:0" would allow the text and the "x" to overlap; I'm not sure if that's good or bad.)

::: toolkit/content/widgets/datetimebox.css:60
(Diff revision 2)
>    border: none;
>    height: 12px;
>    width: 12px;
>    align-self: center;
> -  justify-content: flex-end;
> +  flex: none;
> +  overflow: hidden;

I don't understand what the reason is behind "flex: none; overflow: hidden" here.

If you've got flex:none, then this element should get its desired size (width:12px/height:12px), regardless of space. So, there shouldn't be anything inside of it that would overflow (given that it only exists to paint a background & has no descendants AFAICT).
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review148914
Attachment #8860300 - Flags: review?(dholbert) → review-
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review148884

Thanks Daniel, these are super useful to me, I was just thinking how to test this. I'll try to add some reftests based on your suggestions.

> Right now, you're setting 4 different components to each be "overflow: hidden".
> 
> This one here (for "input") makes sense -- at the edge of the control, where we care about clipping & overflow.  But why are the others necessary?  (Are they necessary?)
> 
> (I can imagine the text-content wrapper might need to be overflow:hidden as well, so that it doesn't overlap the "x" icon when space is limited.  Though: I'm not 100% sure the "x" icon is the most important thing to show in that scenario. Perhaps it'd be better to let the x icon get pushed out of view? *shrug*)

I don't have a strong opinion on this, but it seems that Chrome also always shows the "x" button when space is limited. And this way, you can always reset the fields, no matter how many fields you've already entered, without moving to the last part.
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review148910

> Perhaps "min-width:0" works just as well here?
> 
> (I'll bet you really just want to allow this component to compress to be arbitrarily small -- and for a flex item, "min-width:0" is the simplest way to do that. "overflow:hidden" works too but it's a much larger hammer, because it adds a layer of scrollframe & other complexity.)

I see. `min-width: 0` works well here.

> (I suspect this is the component that wraps the --:-- -- text. Do you really want this to be "overflow:hidden" and forcibly clip its text contents at the spot where the "x" icon would appear?  Or is "min-width:0" [to allow arbitrary shrinking] good enough?  I suspect "min-width:0" would allow the text and the "x" to overlap; I'm not sure if that's good or bad.)

Yes, this wraps the --:-- -- text. I think I'll keep it with `overflow: hidden`. As you mentioned, using `min-width: 0` will make the text and the "x" to overlap, in that case, it feels like there is something behind the "x" but you can not see it.

> I don't understand what the reason is behind "flex: none; overflow: hidden" here.
> 
> If you've got flex:none, then this element should get its desired size (width:12px/height:12px), regardless of space. So, there shouldn't be anything inside of it that would overflow (given that it only exists to paint a background & has no descendants AFAICT).

Right, we don't know need the `overflow: hidden` here, just `flex: none`.
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review148884

> I don't have a strong opinion on this, but it seems that Chrome also always shows the "x" button when space is limited. And this way, you can always reset the fields, no matter how many fields you've already entered, without moving to the last part.

OK; if it's consistent with other browsers' shipping behavior, it seems like a reasonable design choice (RE having dedicated space for the "x").  Thanks for investigating.
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7257eb0052d10755811b44381e2a0e9fe1cec4d

Cancelling review, as some of the reftests fails.

On try theses tests fails:
layout/reftests/forms/input/datetime/time-small-height.html ==
layout/reftests/forms/input/datetime/time-small-height-ref.html

layout/reftests/forms/input/datetime/time-small-width-height.html == layout/reftests/forms/input/datetime/time-small-width-height-ref.html

But locally only this fails:
layout/reftests/forms/input/datetime/time-small-height.html ==
layout/reftests/forms/input/datetime/time-small-height-ref.html


Inspecting the element using Developer Tools, the element in time-small-height.html and time-small-height-ref have exactly the same size (width/height) and look. After decoding the resulting base64 to png, it seems that the ref image is the same but it is positioned a little bit lower. Could this be the case? Do you know how can I fix this?

Thanks.
Flags: needinfo?(dholbert)
Attachment #8860300 - Flags: review?(dholbert)
That looks like we're choosing a different baseline for the <input type="time"> element, as compared to the normal <input> element.  Each element aligns its baseline with a hypothetical run of text that could appear next to it, and depending on where we think the <input type="whatever"> element's baseline is, that puts it at a different y-position.

IIRC, you can hack around that sort of issue by adding "vertical-align:top" to the <input> element, to override baseline alignment.  However, you also should check if this is a regression in this patch.  If this alignment issue is a regression with this patch, then we probably need to address it before landing this.  If it's a separate issue that's independent of this bug, then please file a followup for it and include a code-comment with a mention of that followup bug number alongside any CSS hackaround for this issue in the testcases here.
Flags: needinfo?(dholbert) → needinfo?(jjong)
(In reply to Daniel Holbert [:dholbert] from comment #16)

You're right! It is the about the baseline.

To test if it's a regression in this patch, I used this style:
  input {
    width: 64px;
    height: 5px;
    color: transparent;
  }
for <input> and <input type=time> and checked if they are similar _without_ this patch and... failed. So, I think it's a separate issue, I'll file a follow-up bug for this and add comments in my test files. Thanks for hint.
Flags: needinfo?(jjong)
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review150378

This is almost there! Nits below:

::: commit-message-20dff:1
(Diff revision 4)
> +Bug 1311857 - [DateTimeInput] Set css properties correctly to avoid inner components overflow. r?dholbert

"Set css properties correctly" is a bit vague (and perhaps optimistic -- who's to say this patch has gotten it exactly correct yet? :))

How about:
"Adjust CSS for date/time widget internals to prevent them from overflowing the widget."
...or something like that.

::: layout/reftests/forms/input/datetime/reftest.list:16
(Diff revision 4)
>  # type change
>  skip-if(Android) fails-if(styloVsGecko) == to-time-from-other-type-unthemed.html time-simple-unthemed.html
>  skip-if(Android) == from-time-to-other-type-unthemed.html from-time-to-other-type-unthemed-ref.html
> +
> +# content should not overflow on small width/height
> +skip-if(Android) fails-if(styloVsGecko||stylo) == time-small-width.html time-small-width-ref.html

I'm curious why this reftest (and *only* this one) needs the "||stylo" annotation.  Do you know what's wrong there? This might merit a followup bug or more investigation.

::: layout/reftests/forms/input/datetime/time-content-left-aligned-ref.html:4
(Diff revision 4)
> +     <input type="time" style="width: 200px">
> +     <!-- div to cover the right area -->
> +     <div style="display:block; position:absolute; background-color:black; top:0px; left:40px; width:200px; height:100px;">
> +  </body>
> +</html>

Two nits:
 (1) Please add a </div> so that this is valid HTML. (view-source shouldn't hightlight anything in red.)

 (2) Please keep the lines less than 80 characters long, per coding style guidelines & so they can be viewed in 80-character-wide terminals. You could just add a linebreak after "black", for example.

So to address both of the above, please replace the <div ...> line with these two lines:
     <div style="display:block; position:absolute; background-color:black;
                 top:0px; left:40px; width:200px; height:100px;"></div>

All of the time-content-* and time-reset-* reftest files need a tweak like this.

::: layout/reftests/forms/input/datetime/time-reset-button-right-aligned-ref.html:4
(Diff revision 4)
> +<!DOCTYPE html>
> +<html>
> +  <body>
> +    <input type="time" value ="10:00" style="width: 300px; float: right;">

This reftest "passes" on my Firefox Nightly (on Linux) right now, because the clear button is covered up in both the testcase and the reference case.

To test this effectively, I think you need to:
 (1) remove the "width" from the input in the reference case -- so that it's intrinsically sized & has no extra space inside of it. (so that the "x" will be more likely to actually be right-aligned, even in the face of possible alignment bugs like the one we have in current Nightly)

 (2) make the mask div a few pixels wider, so that it covers up all of the text in the reference case's now-skinnier widget. (It's OK if it has to cover up a little of the "x", too, if that's necessary to be sure you're covering all the text.  We just want to be sure that *some* of the "x" is visible, so that we can ensure that it's also visible in the testcase.)

::: layout/reftests/forms/input/datetime/time-reset-button-right-aligned.html:4
(Diff revision 4)
> +<!DOCTYPE html>
> +<html>
> +  <body>
> +    <input type="time" value ="10:00" style="width: 150px; float: right;">

Typo: s/value =/value=/  (drop the space before the equals sign)

This typo is present in the testcase and the reference case.

::: layout/reftests/forms/input/datetime/time-small-height-ref.html:6
(Diff revision 4)
> +  width: 64px;
> +  height: 5px;

"width 64px" seems quite small here (for a testcase that is explicitly *not* intending to test the effects of a small width)

In addition to being small, it also feels a bit arbitrary/magic.

Could you increase this to 200px? (in the reference case + testcase)  That should be large enough that we won't be width-constrained, and it's not as magic/mysterious-looking.

::: layout/reftests/forms/input/datetime/time-small-width-ref.html:7
(Diff revision 4)
> +<html>
> +  <head>
> +    <style>
> +input {
> +  width: 10px;
> +  height: 15px;

This "height: 15px" feels a bit small & magic, for a testcase that's really intending to test the effects of small widths.

Do you need to specify any particular "height" here at all?  If you can just leave height unset & use the intrinsic height, that'd be preferable...

(But if you do need to specify a height [because e.g. maybe <input type="time"> has a different intrinsic height from <input> on some platform], then something like "height: 1.5em" would be better here.  That should be large enough to not be constraining, yet still short enough that "outline" should still warn us if the contents incorrectly wrap to two lines.)

::: layout/style/res/forms.css:1190
(Diff revision 4)
> +input[type="date"],
> +input[type="time"] {
> +  overflow: hidden;
> +  white-space: nowrap;
> +}

Please add "!important" to these declarations, so that authors cannot override them with e.g.
  data:text/html,<input type="time" style="overflow:visible; width:10px">
(which would make this bug show itself again)

Side note:  I'm curious how/why we we only need this for these widgets & not others.  There must be something that achieves this same clipping for <input type="text">.   Maybe we should really be sharing whatever CSS and/or code is responsible for that?  Or if not sharing, perhaps we should include a comment here that says "This clips the contents like XYZ does for other input elements"...?  This would be worth a little investigation, if you could, so that this special case is more understandable in the context of other elements.

(Another side note: it's a bit problematic to *directly* add CSS to input elements [rather than adding CSS for their secret internal components], because these sorts of tweaks are programmatically detectable by web authors via e.g. calls to getComputedStyle(myInputElem, "").overflowX.  If it's possible to avoid adding our own extra styles for these elements, that'd be ideal.  But I'm not sure that's easy.)

::: toolkit/content/widgets/datetimebox.css:20
(Diff revision 4)
> +  flex: 1;
> +  justify-content: flex-start;

I think you want to get rid of these first two declarations here -- and instead, add this line to the rule above (.datetime-input-box-wrapper):

  justify-content: space-between;

With that:
 - Neither of the flex items (the editable area, the reset button) will absorb any free space beyond their intrinsic size. (Which is good, I think.)
 - The container (datetime-input-box-wrapper) will distribute all of its free space *between the flex items*.
 - And if space is constrained, we'll still steal it from the editable area, since it's still got "overflow:hidden" (the one declaration remaining in this rule after you remove the other two), which lets it shrink below its 
intrinsic size (and clip).
Attachment #8860300 - Flags: review?(dholbert) → review-
(In reply to Daniel Holbert [:dholbert] from comment #20)
> ::: layout/style/res/forms.css:1190
> (Diff revision 4)
> > +input[type="date"],
> > +input[type="time"] {
> > +  overflow: hidden;
> > +  white-space: nowrap;
> > +}
[...]
> (Another side note: it's a bit problematic to *directly* add CSS to input
> elements [rather than adding CSS for their secret internal components],
> because these sorts of tweaks are programmatically detectable by web authors
> via e.g. calls to getComputedStyle(myInputElem, "").overflowX.  If it's
> possible to avoid adding our own extra styles for these elements, that'd be
> ideal.  But I'm not sure that's easy.)

Actually -- maybe you can transplant these overflow/white-space declarations to the .datetime-input-box-wrapper { ... } rule, in datetimebox.css, so that you don't have to touch forms.css at all?  If that works, that would be preferable, from a not-exposing-implementation-details-to-web-content perspective.

(Sorry for not realizing this when I wrote the end of comment 8. At that point I hadn't recalled that this styling would be visible to web content.)
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review150522

::: layout/reftests/forms/input/datetime/time-small-height-ref.html:9
(Diff revision 4)
> +  /* Bug 1370464: [DateTimeInput] Baseline of input box is different from other
> +     inputs (e.g. text) */

As discussed in bug 1370464, it turns out this seems to be a subtle thing where we're behaving as expected.

So: before landing here, could you adjust the various copies of this comment to instead say something more explanatory, like:
  /* Disable baseline alignment, so that our y-position isn't
   * influenced by the choice of font inside of input: */
> (In reply to Daniel Holbert [:dholbert] from comment #20)
> Actually -- maybe you can transplant these overflow/white-space declarations
> to the .datetime-input-box-wrapper { ... } rule, in datetimebox.css, so that
> you don't have to touch forms.css at all?  If that works, that would be
> preferable, from a not-exposing-implementation-details-to-web-content
> perspective.

It doesn't work well If I move overflow/white-space declaration to .datetime-input-box-wrapper { ... } rule, the content is not clipped on the bottom edge of the content. I'll attach a screenshot later.

> 
> (Sorry for not realizing this when I wrote the end of comment 8. At that
> point I hadn't recalled that this styling would be visible to web content.)

No problem, but does that mean the rules in form.css that style the other input elements (not the pseudo elements) have the same problem?
Result of moving overflow/white-space from input[type=date],input[type=time] {} to  .datetime-input-box-wrapper{}.
Attachment #8860308 - Attachment is obsolete: true
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review150378

> "Set css properties correctly" is a bit vague (and perhaps optimistic -- who's to say this patch has gotten it exactly correct yet? :))
> 
> How about:
> "Adjust CSS for date/time widget internals to prevent them from overflowing the widget."
> ...or something like that.

The new commit message looks good to me, I'll update it in the next patch.

> I'm curious why this reftest (and *only* this one) needs the "||stylo" annotation.  Do you know what's wrong there? This might merit a followup bug or more investigation.

Hmm, I decoded the base64 string for the test image and the reference image, but couldn't tell any notable difference, and there is only 6 differing pixels. :(

Here is the try result that fails:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=20e6c37e41a58f1e2e646055f4362b9c5ec95d2d&selectedJob=104776244

> Two nits:
>  (1) Please add a </div> so that this is valid HTML. (view-source shouldn't hightlight anything in red.)
> 
>  (2) Please keep the lines less than 80 characters long, per coding style guidelines & so they can be viewed in 80-character-wide terminals. You could just add a linebreak after "black", for example.
> 
> So to address both of the above, please replace the <div ...> line with these two lines:
>      <div style="display:block; position:absolute; background-color:black;
>                  top:0px; left:40px; width:200px; height:100px;"></div>
> 
> All of the time-content-* and time-reset-* reftest files need a tweak like this.

Will do.

> This reftest "passes" on my Firefox Nightly (on Linux) right now, because the clear button is covered up in both the testcase and the reference case.
> 
> To test this effectively, I think you need to:
>  (1) remove the "width" from the input in the reference case -- so that it's intrinsically sized & has no extra space inside of it. (so that the "x" will be more likely to actually be right-aligned, even in the face of possible alignment bugs like the one we have in current Nightly)
> 
>  (2) make the mask div a few pixels wider, so that it covers up all of the text in the reference case's now-skinnier widget. (It's OK if it has to cover up a little of the "x", too, if that's necessary to be sure you're covering all the text.  We just want to be sure that *some* of the "x" is visible, so that we can ensure that it's also visible in the testcase.)

I updated the test by:
(1) removing the "width" from the input in the reference case
(2) making the mask div position to be a little bit more to the right (`right: 30px`), so that it covers all the text
(3) adding `color: white` to make sure that the text won't influence on the test result

After fixing the test, it fails on stylo run, so I've updated the expected result in reftest.list as well.
Decoding the base64 string from the try result, I can see that the reset icon does not show up at all in the test image, and in the reference image (intrinsically sized), the reset icon does show, but as a weird button.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5273b2538c5ae7b900dc40ea1df40fbc70d9477c&selectedJob=105111253

> Typo: s/value =/value=/  (drop the space before the equals sign)
> 
> This typo is present in the testcase and the reference case.

Will fix them in the next patch, thanks.

> "width 64px" seems quite small here (for a testcase that is explicitly *not* intending to test the effects of a small width)
> 
> In addition to being small, it also feels a bit arbitrary/magic.
> 
> Could you increase this to 200px? (in the reference case + testcase)  That should be large enough that we won't be width-constrained, and it's not as magic/mysterious-looking.

Will do.

> This "height: 15px" feels a bit small & magic, for a testcase that's really intending to test the effects of small widths.
> 
> Do you need to specify any particular "height" here at all?  If you can just leave height unset & use the intrinsic height, that'd be preferable...
> 
> (But if you do need to specify a height [because e.g. maybe <input type="time"> has a different intrinsic height from <input> on some platform], then something like "height: 1.5em" would be better here.  That should be large enough to not be constraining, yet still short enough that "outline" should still warn us if the contents incorrectly wrap to two lines.)

It seems that I need to specify a height, since the intrinsic height of <input type="time"> is different from <input>.
I'll change to use `height: 1.5em` instead.

> Please add "!important" to these declarations, so that authors cannot override them with e.g.
>   data:text/html,<input type="time" style="overflow:visible; width:10px">
> (which would make this bug show itself again)
> 
> Side note:  I'm curious how/why we we only need this for these widgets & not others.  There must be something that achieves this same clipping for <input type="text">.   Maybe we should really be sharing whatever CSS and/or code is responsible for that?  Or if not sharing, perhaps we should include a comment here that says "This clips the contents like XYZ does for other input elements"...?  This would be worth a little investigation, if you could, so that this special case is more understandable in the context of other elements.
> 
> (Another side note: it's a bit problematic to *directly* add CSS to input elements [rather than adding CSS for their secret internal components], because these sorts of tweaks are programmatically detectable by web authors via e.g. calls to getComputedStyle(myInputElem, "").overflowX.  If it's possible to avoid adding our own extra styles for these elements, that'd be ideal.  But I'm not sure that's easy.)

Will add `!important` to these declarations.

I tried to look at how <input> achieves clipping.
Dumping the frames does show a HTMLScroll frame on a inner div, so it must have overflow not set to visible.
But I can only find `overflow: auto;` on `input > .anonymous-div` in forms.css. But didn't `overflow: auto` shows a scrollbar when content is clipped?
Sorry that I'm not very experienced on this, couldn't find out more.

> I think you want to get rid of these first two declarations here -- and instead, add this line to the rule above (.datetime-input-box-wrapper):
> 
>   justify-content: space-between;
> 
> With that:
>  - Neither of the flex items (the editable area, the reset button) will absorb any free space beyond their intrinsic size. (Which is good, I think.)
>  - The container (datetime-input-box-wrapper) will distribute all of its free space *between the flex items*.
>  - And if space is constrained, we'll still steal it from the editable area, since it's still got "overflow:hidden" (the one declaration remaining in this rule after you remove the other two), which lets it shrink below its 
> intrinsic size (and clip).

Adding `justify-content: space-between;` to `.datetime-input-box-wrapper` and removing these two lines here looks good. Will update it in the next patch, thanks.
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review150522

> As discussed in bug 1370464, it turns out this seems to be a subtle thing where we're behaving as expected.
> 
> So: before landing here, could you adjust the various copies of this comment to instead say something more explanatory, like:
>   /* Disable baseline alignment, so that our y-position isn't
>    * influenced by the choice of font inside of input: */

Sure, will adjust the comment where applied.
Hi Daniel, I've uploaded a new patch, but the not-exposing-implementation-details-to-web-content part was not fixed.
(In reply to Jessica Jong [:jessica] from comment #23)
> > (In reply to Daniel Holbert [:dholbert] from comment #20)
> > Actually -- maybe you can transplant these overflow/white-space declarations
> > to the .datetime-input-box-wrapper { ... } rule [snip]
> 
> It doesn't work well If I move overflow/white-space declaration to
> .datetime-input-box-wrapper { ... } rule, the content is not clipped on the
> bottom edge of the content. I'll attach a screenshot later.

Thanks -- from looking at your screenshot, it seems we're failing to shrink datetime-input-box-wrapper to fit the height of the <input> (because nothing tells us todo so, I guess).

It *should* fix things if we added "max-height: 100%" to .datetime-input-box-wrapper -- but my local testing shows that this does not help, I think because .datetime-input-box-wrapper isn't actually given the height of its container in reflow.  I think this is a bug in nsDateTimeControlFrame, though I haven't dived in too deeply yet.

I'm going to poke around a bit more & see if there's an easy fix for that nsDateTimeControlFrame issue, so that we can use max-height:100% & keep this styling private to avoid exposing implementation details.

> does that mean the rules in form.css that style the other
> input elements (not the pseudo elements) have the same problem?

To a varying extent, yes.  If there's a web standard that says a particular element should have a particular CSS style (and there is such a standard in some cases, I think/assume), THEN of course we're fine to include these styles in forms.css.  Beyond that, though, any author-visible CSS in UA stylesheets can create a potential interop issue.

Authors should be able to assume that any unstyled element will have all CSS properties set to their initial value (except where a web standard sets some special default value for that element, as noted above).  And authors should also be able to assume that any CSS that *they* set on these elements will take effect, at least as far as they can tell via getComputedStyle.  So, any User-Agent Stylesheet CSS that violates these author expectations is kind of problematic.  (It may still be worth having, if it's absolutely necessary to avoid other sorts of bugs.  So in this case, I'm wondering whether this CSS is absolutely necessary or if we can achieve the same result via another means.)

(In reply to Jessica Jong [:jessica] from comment #25)
> Hmm, I decoded the base64 string for the test image and the reference image,
> but couldn't tell any notable difference, and there is only 6 differing
> pixels. :(
> 
> Here is the try result that fails:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=20e6c37e41a58f1e2e646055f4362b9c5ec95d2d&selectedJob=104776244

Note: you don't have to decode the base64 string manually to compare images -- you should be using the "reftest-analyzer" tool, which is a button-click from TreeHerder.  Visit that ^^ treeherder page, click an orange "R5", and then click the bar-graph icon (the 5th icon from the left on the bottom bar's header, kind of above "State: completed").  After that loads, you can click the link for the failing reftest, and use the "Circle differences" checkbox to see which pixels differ. (and see the differing RGB values if your aim is good and you hover just right)

Anyway -- in this case, it looks like the internals of the <input> are mostly rgb(251,251,251) or therabouts (not quite white), and the testcase has two rows of pure-white pixels where the reference case does not.  I'll bet these are from the first two "--" characters (which we're styling with "color: white".  These characters aren't present in the reference case, becasue it just has an <input> element.

If you add "background: white", I'll bet that'll fix this. (It'll also remove native styling of the widget, but that's OK, because the native styling is breaking an assumption that we're relying on for this test -- that white text should be undetectable.)
(In reply to Daniel Holbert [:dholbert] from comment #29)
> It *should* fix things if we added "max-height: 100%" to
> .datetime-input-box-wrapper -- but my local testing shows that this does not
> help, I think because .datetime-input-box-wrapper isn't actually given the
> height of its container in reflow.  I think this is a bug in
> nsDateTimeControlFrame, though I haven't dived in too deeply yet.
> 
> I'm going to poke around a bit more & see if there's an easy fix for that
> nsDateTimeControlFrame issue, so that we can use max-height:100% & keep this
> styling private to avoid exposing implementation details.

Hmm; I'm realizing the sort of fix I had in mind still wouldn't really help if we had
 <input type="time" style="max-height:5px">
because then, a hypothetical "height:100%" on the internal .datetime-input-box-wrapper would not be able to figure out that it should resolve to 5px.

Mayyybe "height: 100%; max-height: inherit" might work, but I'm not sure. So for now, I'm OK sticking with the overflow:hidden on the <input> itself and leaving this as a slight wart on our implementation.

(Incidentally, Chrome's devtools show that they also have "overflow:hidden" for this element in a UA stylesheet; so we won't be entirely on our own.  Though they don't have !important, so we're not entirely consistent with them -- and as a result, Chrome's rendering does have overflow from the the input box, at small widget-heights.)
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review151028

::: layout/reftests/forms/input/datetime/reftest.list:16
(Diff revision 5)
>  # type change
>  skip-if(Android) fails-if(styloVsGecko) == to-time-from-other-type-unthemed.html time-simple-unthemed.html
>  skip-if(Android) == from-time-to-other-type-unthemed.html from-time-to-other-type-unthemed-ref.html
> +
> +# content should not overflow on small width/height
> +skip-if(Android) fails-if(styloVsGecko||stylo) == time-small-width.html time-small-width-ref.html

Per last chunk in comment 29, I'll bet you can drop the ||stylo if you add "background: white" to the <input> in the testcase & reference case here.  

(At least, that should avoid the issue shown in the reftest log that you linked, where the testcase has a white "--" on a nearly-white background.)

::: layout/reftests/forms/input/datetime/reftest.list:24
(Diff revision 5)
> +
> +# content (text) should be left aligned
> +skip-if(Android) fails-if(styloVsGecko) == time-content-left-aligned.html time-content-left-aligned-ref.html
> +
> +# reset button should be right aligned
> +skip-if(Android) fails-if(styloVsGecko||stylo) == time-reset-button-right-aligned.html time-reset-button-right-aligned-ref.html

(For reference, this ||stylo failure is for
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5273b2538c5ae7b900dc40ea1df40fbc70d9477c&selectedJob=105111253 )

This looks a bit mysterious (given that all the other old + new tests in this directory pass with stylo, IIUC).

Please file a followup for this ||stylo failure, & leave a comment in the reftest.list file saying something like
 # Bug NNN covers the stylo failure here.

::: layout/style/res/forms.css:1190
(Diff revision 5)
> +input[type="date"],
> +input[type="time"] {
> +  overflow: hidden !important;
> +  white-space: nowrap !important;
> +}

Per comment 30, I'm OK with "overflow:hidden" line here for now, since it's complicated get the clipping right if we try to abstract that away to an internal component.

BUT, I don't think we need to expose this "white-space" declaration here -- it can move to a lower level of granularity.  Can you push that style down as far as possible? (maybe to ".datetime-edit-field", which IIUC is the container for the actual text, or a closer ancestor of the text at least.)  After it's on a pseudo-element, it can lose the !important, too, since it won't be author-overridable.

::: toolkit/content/widgets/datetimebox.css:11
(Diff revision 5)
>  @namespace xul url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
>  
>  .datetime-input-box-wrapper {
>    -moz-appearance: none;
>    display: inline-flex;
> +  flex: 1;

Why are you adding this "flex: 1"? I don't expect that makes a difference -- "flex" only has an effect on *children* of a flex container.  (And this element is not a child of a flex container, as far as I can tell.  It *is* the flex container.)

::: toolkit/content/widgets/datetimebox.css:16
(Diff revision 5)
> +  flex: 1;
>    cursor: default;
>    background-color: inherit;
>    color: inherit;
>    font-family: monospace;
> +  min-width: 0;

Similarly, since this element isn't a child of a flex container (IIUC), you shouldn't need "min-width: 0" here -- that style shouldn't make a difference.

The default min-width value ("auto") behaves like 0 in all cases *except* for flex/grid children.
Attachment #8860300 - Flags: review?(dholbert) → review-
(In reply to Daniel Holbert [:dholbert] from comment #29)
> Authors should be able to assume that any unstyled element will have all CSS
> properties set to their initial value (except where a web standard sets some
> special default value for that element, as noted above).  And authors should
> also be able to assume that any CSS that *they* set on these elements will
> take effect, at least as far as they can tell via getComputedStyle.  So, any
> User-Agent Stylesheet CSS that violates these author expectations is kind of
> problematic.  (It may still be worth having, if it's absolutely necessary to
> avoid other sorts of bugs.  So in this case, I'm wondering whether this CSS
> is absolutely necessary or if we can achieve the same result via another
> means.)

I understand now, thanks for your explanation.

> Note: you don't have to decode the base64 string manually to compare images
> -- you should be using the "reftest-analyzer" tool, which is a button-click
> from TreeHerder.  Visit that ^^ treeherder page, click an orange "R5", and
> then click the bar-graph icon (the 5th icon from the left on the bottom
> bar's header, kind of above "State: completed").  After that loads, you can
> click the link for the failing reftest, and use the "Circle differences"
> checkbox to see which pixels differ. (and see the differing RGB values if
> your aim is good and you hover just right)

Wow, I dind't know about this handy tool, very useful, thanks.

> 
> If you add "background: white", I'll bet that'll fix this. (It'll also
> remove native styling of the widget, but that's OK, because the native
> styling is breaking an assumption that we're relying on for this test --
> that white text should be undetectable.)

Adding "background: white" fixes the testcase, thanks.

(In reply to Daniel Holbert [:dholbert] from comment #30)
> Hmm; I'm realizing the sort of fix I had in mind still wouldn't really help
> if we had
>  <input type="time" style="max-height:5px">
> because then, a hypothetical "height:100%" on the internal
> .datetime-input-box-wrapper would not be able to figure out that it should
> resolve to 5px.
> 
> Mayyybe "height: 100%; max-height: inherit" might work, but I'm not sure. So
> for now, I'm OK sticking with the overflow:hidden on the <input> itself and
> leaving this as a slight wart on our implementation.
> 
> (Incidentally, Chrome's devtools show that they also have "overflow:hidden"
> for this element in a UA stylesheet; so we won't be entirely on our own. 
> Though they don't have !important, so we're not entirely consistent with
> them -- and as a result, Chrome's rendering does have overflow from the the
> input box, at small widget-heights.)

Thanks for helping on this, I'll keep the `overflow:hidden` on the <input> itself for now.
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review151028

> Per last chunk in comment 29, I'll bet you can drop the ||stylo if you add "background: white" to the <input> in the testcase & reference case here.  
> 
> (At least, that should avoid the issue shown in the reftest log that you linked, where the testcase has a white "--" on a nearly-white background.)

Thanks for catching this, adding `background: white` solves both styloVsGecko and stylo testcases.

> (For reference, this ||stylo failure is for
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5273b2538c5ae7b900dc40ea1df40fbc70d9477c&selectedJob=105111253 )
> 
> This looks a bit mysterious (given that all the other old + new tests in this directory pass with stylo, IIUC).
> 
> Please file a followup for this ||stylo failure, & leave a comment in the reftest.list file saying something like
>  # Bug NNN covers the stylo failure here.

I noticed this warning in the log:
"WARNING: stylo: cannot get ServoStyleSheets from XBL bindings yet. See bug 1290276."

So maybe it's because our styling in datetimebox.css does not take effect.

(After moving `white-space: nowrap` to datetimebox.css, time-content-left-aligned* fails as well, see below)

> Per comment 30, I'm OK with "overflow:hidden" line here for now, since it's complicated get the clipping right if we try to abstract that away to an internal component.
> 
> BUT, I don't think we need to expose this "white-space" declaration here -- it can move to a lower level of granularity.  Can you push that style down as far as possible? (maybe to ".datetime-edit-field", which IIUC is the container for the actual text, or a closer ancestor of the text at least.)  After it's on a pseudo-element, it can lose the !important, too, since it won't be author-overridable.

I have moved `white-space: nowrap` to datetime-input-edit-wrapper {...}, which is the wrapper for all the text fields; datetime-edit-field is for each of the individual text field.

However, moving to there makes time-content-left-aligned* test fail, since the <input type=time> in time-content-left-aligned.html has a small width, hence breaks into two lines. It seems that the styling in datetimebox.css does not take effect, because of this:

"WARNING: stylo: cannot get ServoStyleSheets from XBL bindings yet. See bug 1290276."

But, time-small-width* passes, I guess it's because <input> suffers the same problem as <input type=time>

> Why are you adding this "flex: 1"? I don't expect that makes a difference -- "flex" only has an effect on *children* of a flex container.  (And this element is not a child of a flex container, as far as I can tell.  It *is* the flex container.)

I think `datetime-input-box-wrapper` is a flex container and a flex item at the same time. We added `display: flex` to the xul element itself.
http://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/layout/style/res/html.css#789

Adding `flex: 1` makes the whole thing (all the contents defined here) to take up all the space, right?

> Similarly, since this element isn't a child of a flex container (IIUC), you shouldn't need "min-width: 0" here -- that style shouldn't make a difference.
> 
> The default min-width value ("auto") behaves like 0 in all cases *except* for flex/grid children.

As explained above, if this is a flex item, `min-width: 0` allow this component to compress to be arbitrarily small, as you suggested before, right?
(In reply to Jessica Jong [:jessica] from comment #33)
> Comment on attachment 8860300 [details]
> I noticed this warning in the log:
> "WARNING: stylo: cannot get ServoStyleSheets from XBL bindings yet. See bug
> 1290276."
> 
> So maybe it's because our styling in datetimebox.css does not take effect.

Ah, OK. Maybe it doesn't deserve a dedicated followup bug then, and it's just a special case of that more general known-and-hopefully-soon-to-be-fixed issue.  Could you include a comment in reftest.list above these tests though, saying something like...
 # Stylo failures below are likely b/c bug 1290276 prevents datetimebox.css
 # from being honored. 

> I think `datetime-input-box-wrapper` is a flex container and a flex item at
> the same time. We added `display: flex` to the xul element itself.
> http://searchfox.org/mozilla-central/rev/
> 1a054419976437d0778a2b89be1b00207a744e94/layout/style/res/html.css#789

I see - sorry for missing that, & thanks for clarifying.  Given that, the "flex:1" and "min-width:0" styling makes sense, yes. Thanks!

> I have moved `white-space: nowrap` [...]
> 
> However, moving to there makes time-content-left-aligned* test fail,
> [...] ""WARNING: stylo:"

So to be clear, this test-failure is stylo-only, right? If so, I think this is fine, & it should be annotated as a legitimate test failure (and the reftest.list comment that I suggested above would serve nicely as an explanation for it).
Oh, actually -- it looks like bug 1290276 (the bug referenced by the ServoStyleSheets/xbl WARNING message) actually *just* landed on autoland (6 hours ago)!

So, you should probably rebase on top of that bug's changes (e.g. by doing "hg pull https://hg.mozilla.org/integration/autoland/" in your normal tree, if the csets haven't made it to m-c yet... This is assuming https://treeherder.mozilla.org/#/jobs?repo=autoland looks green enough when you do this), and optimistically try removing your stylo test-failure annotations, an hopefully everything will pass at this point.

And if stuff's still broken, then we probably do need a followup bug to cover any remaining stylo failures, because the "WARNING" that we were previously blaming should no longer exist, now that bug 1290276 has landed.
Flags: needinfo?(jjong)
(In reply to Daniel Holbert [:dholbert] from comment #35)
> Oh, actually -- it looks like bug 1290276 (the bug referenced by the
> ServoStyleSheets/xbl WARNING message) actually *just* landed on autoland (6
> hours ago)!
> 
> So, you should probably rebase on top of that bug's changes (e.g. by doing
> "hg pull https://hg.mozilla.org/integration/autoland/" in your normal tree,
> if the csets haven't made it to m-c yet... This is assuming
> https://treeherder.mozilla.org/#/jobs?repo=autoland looks green enough when
> you do this), and optimistically try removing your stylo test-failure
> annotations, an hopefully everything will pass at this point.
> 
> And if stuff's still broken, then we probably do need a followup bug to
> cover any remaining stylo failures, because the "WARNING" that we were
> previously blaming should no longer exist, now that bug 1290276 has landed.

Aha! Just in time. I ran try again on top of latest m-c (bug 1290276 included), stylo tests now pass, but time-reset-button-right-aligned* fails with styloVsGecko.

Looking at the result,
https://treeherder.mozilla.org/#/jobs?repo=try&revision=82d9a5185e829856549f93472d391ae33ab4b686&selectedJob=105720707

it seems that the reset button is not rendered correctly when set a custom width.
So, I'll keep the fails-is(styloVsGecko) statement in time-reset-button-right-aligned*, do you think this needs a dedicated follow-up bug?
Flags: needinfo?(jjong)
That's odd!

It's possible that's covered by some other known Stylo missing-feature, but in the absence of knowledge of such a bug, please do file a bug to track this failure and mention it in a comment at the end of the reftest.list line for this testcase.

(general rule: all expected test-failures should have an associated tracking bug, which should ideally be linked alongside the failure annotation.)
(In reply to Jessica Jong [:jessica] from comment #36)
> (In reply to Daniel Holbert [:dholbert] from comment #35)
> it seems that the reset button is not rendered correctly when set a custom
> width.

(Actually -- for all we know, the reset button is not rendered correctly *ever* in Stylo, right?  If this is the only reftest that has the reset button showing [and I think it might be], then that's the simplest conclusion, at least.)
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review151894

r=me

::: layout/reftests/forms/input/datetime/reftest.list:24
(Diff revisions 5 - 6)
>  
>  # content (text) should be left aligned
> -skip-if(Android) fails-if(styloVsGecko) == time-content-left-aligned.html time-content-left-aligned-ref.html
> +skip-if(Android) == time-content-left-aligned.html time-content-left-aligned-ref.html
>  
>  # reset button should be right aligned
> -skip-if(Android) fails-if(styloVsGecko||stylo) == time-reset-button-right-aligned.html time-reset-button-right-aligned-ref.html
> +skip-if(Android) fails-if(styloVsGecko) == time-reset-button-right-aligned.html time-reset-button-right-aligned-ref.html

As discussed -- before landing, please do file a Core|CSS bug for this styloVsGecko failure and add a "# bug NNN" annotation at the end of this line.

(The bug doesn't need to be super-detailed -- just include a link to the failing Try run & mention that the failure seems to happen because Stylo seems to paint the reset button as a normal button rather than with a custom background [IIUC].  Please CC me and a stylo developer -- maybe Xidorn or heycam. They may know whether this is mysterious or an instance of a more general known issue.)
Attachment #8860300 - Flags: review?(dholbert) → review+
Comment on attachment 8860300 [details]
Bug 1311857 - Adjust CSS for date/time widget internals to prevent them from overflowing the widget.

https://reviewboard.mozilla.org/r/132300/#review151894

> As discussed -- before landing, please do file a Core|CSS bug for this styloVsGecko failure and add a "# bug NNN" annotation at the end of this line.
> 
> (The bug doesn't need to be super-detailed -- just include a link to the failing Try run & mention that the failure seems to happen because Stylo seems to paint the reset button as a normal button rather than with a custom background [IIUC].  Please CC me and a stylo developer -- maybe Xidorn or heycam. They may know whether this is mysterious or an instance of a more general known issue.)

Actually, since the merge is imminent and you're probably asleep for the next N hours and then on weekend (and then it's train-merge-time): I'm gonna go ahead and autoland this on your behalf, in its current state -- but please do file the followup bug and push a "DONTBUILD" followup changeset to inbound at some point soon [once this is merged there], to add a bug number for the styloVsGecko failure here.
Flags: needinfo?(jjong)
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f3cf37e0c74
Adjust CSS for date/time widget internals to prevent them from overflowing the widget. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/4f3cf37e0c74
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(In reply to Daniel Holbert [:dholbert] from comment #41)
> Actually, since the merge is imminent and you're probably asleep for the
> next N hours and then on weekend (and then it's train-merge-time): I'm gonna
> go ahead and autoland this on your behalf, in its current state -- but
> please do file the followup bug and push a "DONTBUILD" followup changeset to
> inbound at some point soon [once this is merged there], to add a bug number
> for the styloVsGecko failure here.

Thanks Daniel for autolanding it, I filed Bug 1372062, and will upload the follow-up patch now.
Flags: needinfo?(jjong)
Pushed by jjong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b93a54ef243a
(followup) Add bug number for expected test failure in datetime reftest.list. r=me, comment-only, DONTBUILD
Depends on: 1372062
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: