Closed Bug 1317581 Opened 7 years ago Closed 6 years ago

[DateTimePicker] Picker should support high contrast mode on Windows

Categories

(Toolkit :: Themes, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: scottwu, Assigned: jaws)

References

(Blocks 2 open bugs)

Details

(Keywords: access)

Attachments

(2 files, 1 obsolete file)

      No description provided.
Currently the time picker doesn't work under high contrast mode. I believe the reason is that I had defined colors based on the visual spec, instead of using the color palette defined in Firefox.

I wonder if there a complete list of system color palette I can reference?

And what can I do if there is no system color that matches the ones used in visual spec? Does the visual spec need to conform to the system colors that are available?


Datetime picker visual spec: https://bugzilla.mozilla.org/show_bug.cgi?id=1069609#c60
Colors defined in stylesheet: http://searchfox.org/mozilla-central/rev/886d5186f0598ab2f8a9953eb5a4dab9750ef834/toolkit/themes/shared/timepicker.css#15-26
Assignee: nobody → scwwu
Flags: needinfo?(dao+bmo)
(In reply to Scott Wu [:scottwu] from comment #1)
> Currently the time picker doesn't work under high contrast mode. I believe
> the reason is that I had defined colors based on the visual spec, instead of
> using the color palette defined in Firefox.
> 
> I wonder if there a complete list of system color palette I can reference?

There's https://developer.mozilla.org/de/docs/Web/CSS/color#deprecated-system-color but it's not complete.

https://dxr.mozilla.org/mozilla-central/source/widget/tests/test_platform_colors.xul#25 contains mozilla extensions but I don't know if it's complete either.

https://dxr.mozilla.org/mozilla-central/source/layout/style/nsCSSKeywordList.h#38 should be complete but lists more than just colors.

> And what can I do if there is no system color that matches the ones used in
> visual spec?

You can use semi-transparent colors such as hsla(0,0%,0%,.1) or hsla(255,0%,0%,.1) and try to match the spec while still looking reasonable with different OS themes, but this may not always be viable. You can also restrict hardcoded colors to the default theme on Windows using @media (-moz-windows-default-theme), but then you'll still need a reasonable fallback for High Contrast mode (as well as for Linux where we don't have a uniform default OS theme.)

> Does the visual spec need to conform to the system colors that
> are available?

Ideally a visual spec should take stuff like this into consideration from the start, yes.
Flags: needinfo?(dao+bmo)
Component: XUL Widgets → Themes
Priority: -- → P2
Whiteboard: [milestone5]
Summary: [DateTimePicker] Time picker should support high contrast mode on Windows → [DateTimePicker] Picker should support high contrast mode on Windows
Priority: P2 → P3
Whiteboard: [milestone5]
Keywords: access
Assignee: scottcwwu → jaws
Status: NEW → ASSIGNED
Attachment #8961466 - Attachment is obsolete: true
Attachment #8961466 - Flags: review?(dao+bmo)
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

https://reviewboard.mozilla.org/r/230260/#review235858

::: toolkit/content/widgets/datetimebox.css:52
(Diff revision 1)
>  }
>  
>  .datetime-reset-button {
>    background-image: url(chrome://global/skin/icons/input-clear.svg);
> +  -moz-context-properties: fill;
> +  fill: GrayText;

With and without this patch this button is not visible on High Contrast Dark #1. @Dao, do you see what is wrong here?
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> Comment on attachment 8961486 [details]
> Bug 1317581 - DateTimePicker should support high contrast mode on Windows.
> 
> https://reviewboard.mozilla.org/r/230260/#review235858
> 
> ::: toolkit/content/widgets/datetimebox.css:52
> (Diff revision 1)
> >  }
> >  
> >  .datetime-reset-button {
> >    background-image: url(chrome://global/skin/icons/input-clear.svg);
> > +  -moz-context-properties: fill;
> > +  fill: GrayText;
> 
> With and without this patch this button is not visible on High Contrast Dark
> #1. @Dao, do you see what is wrong here?

input-clear.svg doesn't seem to be using context-fill?
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

https://reviewboard.mozilla.org/r/230260/#review235862

::: toolkit/themes/shared/icons/input-clear.svg:12
(Diff revision 1)
> -    .st0 {
> -      fill: #858585;
> -    }
> -    .st1 {
> -      fill: #FFFFFF;
> -    }
> +    <mask id="x">
> +      <circle fill="#fff" cx="6" cy="6" r="6"/>
> +      <polygon points="9,8.1 8.1,9 6,6.9 3.9,9 3,8.1 5.1,6 3,3.9 3.9,3 6,5.1 8.1,3 9,3.9 6.9,6" fill="#000"/>
> +    </mask>
> +  </defs>
> +  <circle fill="context-fill" cx="6" cy="6" r="6" mask="url(#x)"/>

It's using context-fill here.
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

So the date and time pickers are HTML documents in an iframe in a panel (*sigh*). I'm guessing these documents are considered content rather than chrome, and thus context properties are disabled there.
Attachment #8961486 - Flags: review?(dao+bmo)
Btw, I just tried this patch on Windows 10 (default theme) and I'm not sure I'm seeing the expected outcome. Weekends are still red and I assume they should be blue?
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

https://reviewboard.mozilla.org/r/230260/#review240202

::: browser/themes/shared/browser.inc.css:146
(Diff revision 1)
>  /* End private browsing and accessibility indicators */
>  
>  /* Force background for datepicker popup to white so themes don't override it */
> -#DateTimePickerPanel[active="true"] > .panel-arrowcontainer > .panel-arrowbox,
> -#DateTimePickerPanel[active="true"] > .panel-arrowcontainer > .panel-arrowcontent {
> -  --arrowpanel-background: #fff;
> +#DateTimePickerPanel[active="true"] {
> +  --arrowpanel-background: Window;
> +  --arrowpanel-border-color: ThreeDShadow;

It would be nice if this actually used the correct values from popup.css.
Attachment #8961486 - Flags: review?(dao+bmo)
Attachment #8961486 - Flags: review?(dao+bmo)
(In reply to Dão Gottwald [::dao] from comment #9)
> Btw, I just tried this patch on Windows 10 (default theme) and I'm not sure
> I'm seeing the expected outcome. Weekends are still red and I assume they
> should be blue?

They were red before the patch so I kept them red. I used data:text/html,<input type="date"> to test it. The previous code had `--weekend-font-color: rgb(218, 78, 68);` which is a shade of red/pink.
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

https://reviewboard.mozilla.org/r/230260/#review235858

> With and without this patch this button is not visible on High Contrast Dark #1. @Dao, do you see what is wrong here?

Thanks for your comments. I didn't immediately understand what you were getting at but I figured it out eventually. I've moved the SVG to be inline which allows us to use fillGrayText and -moz-field appropriately. When using those fill values withthe external SVG I only got black. This now works with High Contrast Dark #1.
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

https://reviewboard.mozilla.org/r/230260/#review240862

::: browser/themes/shared/browser.inc.css:146
(Diff revision 3)
>  /* End private browsing and accessibility indicators */
>  
>  /* Force background for datepicker popup to white so themes don't override it */
> -#DateTimePickerPanel[active="true"] > .panel-arrowcontainer > .panel-arrowbox,
> -#DateTimePickerPanel[active="true"] > .panel-arrowcontainer > .panel-arrowcontent {
> -  --arrowpanel-background: #fff;
> +#DateTimePickerPanel[active="true"] {
> +  --arrowpanel-background: Menu;
> +  --arrowpanel-border-color: ThreeDShadow;

You seem to have misunderstood comment 10. 
I meant these:

https://searchfox.org/mozilla-central/rev/b55e1a1cbcaee34878e133fbac20c4c2af6e11b5/toolkit/themes/linux/global/popup.css#37-38

https://searchfox.org/mozilla-central/rev/b55e1a1cbcaee34878e133fbac20c4c2af6e11b5/toolkit/themes/osx/global/popup.css#45,47

https://searchfox.org/mozilla-central/rev/b55e1a1cbcaee34878e133fbac20c4c2af6e11b5/toolkit/themes/windows/global/popup.css#49,51

And the crucial part here is that the variable defaults vary across platforms.

::: toolkit/content/widgets/datetimebox.css:53
(Diff revision 3)
>  
>  .datetime-reset-button {
> -  background-image: url(chrome://global/skin/icons/input-clear.svg);
> +  fill: GrayText;
>    background-color: transparent;
>    background-repeat: no-repeat;
>    background-size: 12px, 12px;

No point in specifying background-repeat and background-size now.

::: toolkit/content/widgets/datetimebox.xml:1222
(Diff revision 3)
>          <html:button class="datetime-reset-button" anonid="reset-button"
> -                     tabindex="-1" xbl:inherits="disabled" aria-label="&datetime.reset.label;"/>
> +                     tabindex="-1" xbl:inherits="disabled" aria-label="&datetime.reset.label;">
> +          <svg xmlns="http://www.w3.org/2000/svg" width="12" height="12">
> +            <circle cx="6" cy="6" r="6"/>
> +            <polygon points="9,8.1 8.1,9 6,6.9 3.9,9 3,8.1 5.1,6 3,3.9 3.9,3 6,5.1 8.1,3 9,3.9 6.9,6"
> +                     fill="-moz-field"/>

This should probably use a mask to cut into the circle rather than overlaying it with another color.
Attachment #8961486 - Flags: review?(dao+bmo)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> (In reply to Dão Gottwald [::dao] from comment #9)
> > Btw, I just tried this patch on Windows 10 (default theme) and I'm not sure
> > I'm seeing the expected outcome. Weekends are still red and I assume they
> > should be blue?
> 
> They were red before the patch so I kept them red. I used
> data:text/html,<input type="date"> to test it. The previous code had
> `--weekend-font-color: rgb(218, 78, 68);` which is a shade of red/pink.

I must still be missing something. Your patch sets -weekend-font-color: -moz-activehyperlinktext, so how exactly are you keeping them red?
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

https://reviewboard.mozilla.org/r/230260/#review241306

::: toolkit/themes/shared/datetimeinputpickers.css:18
(Diff revision 4)
>    --day-period-spacing-width: 1rem;
>    --calendar-width: 23.1rem;
>    --date-picker-item-height: 2.5rem;
>    --date-picker-item-width: 3.3rem;
>  
> -  --border: 0.1rem solid #D6D6D6;
> +  --border: 0.1rem solid ThreeDLightShadow;

ThreeDLightShadow is kind of broken (as in mostly invisible) on Linux. Can you use ThreeDShadow?

::: toolkit/themes/shared/datetimeinputpickers.css:20
(Diff revision 4)
>    --date-picker-item-height: 2.5rem;
>    --date-picker-item-width: 3.3rem;
>  
> -  --border: 0.1rem solid #D6D6D6;
> +  --border: 0.1rem solid ThreeDLightShadow;
>    --border-radius: 0.3rem;
> -  --border-active-color: #B1B1B1;
> +  --border-active-color: ActiveBorder;

ActiveBorder is the outer window border. It doesn't make sense to use it in this context.
Attachment #8961486 - Flags: review?(dao+bmo)
Comment on attachment 8966688 [details]
Bug 1317581 - Clean up some duplicated colors in the DateTimePicker.

https://reviewboard.mozilla.org/r/235388/#review241308
Attachment #8966688 - Flags: review?(dao+bmo) → review+
Comment on attachment 8961486 [details]
Bug 1317581 - DateTimePicker should support high contrast mode on Windows.

https://reviewboard.mozilla.org/r/230260/#review243312
Attachment #8961486 - Flags: review?(dao+bmo) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s e086a0c53c054ed13c24935864b200021ec29ee6 -d 697d0f7076eb: rebasing 459133:e086a0c53c05 "Bug 1317581 - DateTimePicker should support high contrast mode on Windows. r=dao"
merging browser/components/extensions/ExtensionPopups.jsm
merging browser/themes/linux/customizableui/panelUI.css
merging browser/themes/shared/customizableui/panelUI.inc.css
merging browser/themes/windows/customizableui/panelUI.css
merging toolkit/modules/LightweightThemeConsumer.jsm
warning: conflicts while merging browser/components/extensions/ExtensionPopups.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/963466173a74
DateTimePicker should support high contrast mode on Windows. r=dao
https://hg.mozilla.org/integration/autoland/rev/5cc146254899
Clean up some duplicated colors in the DateTimePicker. r=dao
Backed out 2 changesets (bug 1317581) for Browser chrome failures on toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js. CLOSED TREE

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5cc1462548991de3dc524484f008e75394cb7df4

Backout: https://hg.mozilla.org/integration/autoland/rev/b1dbb3ffc3d228a345266b03b1e90cb950ac6f40

Failure log:
task 2018-04-18T14:36:15.752Z] 14:36:15     INFO - TEST-START | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js
[task 2018-04-18T14:36:15.800Z] 14:36:15     INFO - GECKO(3690) | ++DOCSHELL 0x7f221ae2c000 == 2 [pid = 3770] [id = {1414dce0-05bf-4895-8f01-4d42151ec843}]
[task 2018-04-18T14:36:15.801Z] 14:36:15     INFO - GECKO(3690) | ++DOMWINDOW == 3 (0x7f2216c2e400) [pid = 3770] [serial = 3] [outer = (nil)]
[task 2018-04-18T14:36:15.879Z] 14:36:15     INFO - GECKO(3690) | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2018-04-18T14:36:15.935Z] 14:36:15     INFO - GECKO(3690) | ++DOMWINDOW == 4 (0x7f2216c2f000) [pid = 3770] [serial = 4] [outer = 0x7f2216c2e400]
[task 2018-04-18T14:36:16.254Z] 14:36:16     INFO - GECKO(3690) | ++DOCSHELL 0x7f2ed3655800 == 1 [pid = 3855] [id = {afddf338-023b-48b1-8cd8-9b20e0bef780}]
[task 2018-04-18T14:36:16.255Z] 14:36:16     INFO - GECKO(3690) | [Parent 3690, Main Thread] WARNING: Could not get disk status from nsIDiskSpaceWatcher: file /builds/worker/workspace/build/src/uriloader/prefetch/nsOfflineCacheUpdateService.cpp, line 291
[task 2018-04-18T14:36:16.336Z] 14:36:16     INFO - GECKO(3690) | ++DOMWINDOW == 1 (0x7f2ed360a000) [pid = 3855] [serial = 1] [outer = (nil)]
[task 2018-04-18T14:36:16.578Z] 14:36:16     INFO - GECKO(3690) | ++DOMWINDOW == 2 (0x7f2ed679bc00) [pid = 3855] [serial = 2] [outer = 0x7f2ed360a000]
[task 2018-04-18T14:36:16.958Z] 14:36:16     INFO - GECKO(3690) | ++DOMWINDOW == 5 (0x7f2216c33800) [pid = 3770] [serial = 5] [outer = 0x7f2216c2e400]
[task 2018-04-18T14:36:17.488Z] 14:36:17     INFO - GECKO(3690) | [Parent 3690, Main Thread] WARNING: NS_ENSURE_TRUE(aSecondURI) failed: file /builds/worker/workspace/build/src/dom/base/ThirdPartyUtil.cpp, line 98
[task 2018-04-18T14:36:17.647Z] 14:36:17     INFO - GECKO(3690) | [Parent 3690, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /builds/worker/workspace/build/src/netwerk/protocol/res/SubstitutingProtocolHandler.cpp, line 338
[task 2018-04-18T14:36:18.277Z] 14:36:18     INFO - TEST-INFO | started process screentopng
[task 2018-04-18T14:36:18.878Z] 14:36:18     INFO - TEST-INFO | screentopng: exit 0
[task 2018-04-18T14:36:18.879Z] 14:36:18     INFO - Buffered messages logged at 14:36:15
[task 2018-04-18T14:36:18.879Z] 14:36:18     INFO - Entering test bound test_popup_styling
[task 2018-04-18T14:36:18.881Z] 14:36:18     INFO - Extension loaded
[task 2018-04-18T14:36:18.881Z] 14:36:18     INFO - Buffered messages logged at 14:36:17
[task 2018-04-18T14:36:18.882Z] 14:36:18     INFO - Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 170}]
[task 2018-04-18T14:36:18.883Z] 14:36:18     INFO - Buffered messages logged at 14:36:18
[task 2018-04-18T14:36:18.884Z] 14:36:18     INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js | Popup background color should have been themed - "rgb(255, 0, 0)" == "rgb(255, 0, 0)" - 
[task 2018-04-18T14:36:18.885Z] 14:36:18     INFO - Buffered messages finished
[task 2018-04-18T14:36:18.887Z] 14:36:18     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js | Popup text color should have been themed - "rgb(60, 60, 60)" == "rgb(0, 128, 0)" - JS frame :: chrome://mochitests/content/browser/toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js :: test_popup_styling/< :: line 60
[task 2018-04-18T14:36:18.889Z] 14:36:18     INFO - Stack trace:
[task 2018-04-18T14:36:18.890Z] 14:36:18     INFO - chrome://mochitests/content/browser/toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js:test_popup_styling/<:60
[task 2018-04-18T14:36:18.892Z] 14:36:18     INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js | Element left border color should be set. - "rgb(0, 0, 255)" == "rgb(0, 0, 255)" - 
[task 2018-04-18T14:36:18.893Z] 14:36:18     INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js | Element right border color should be set. - "rgb(0, 0, 255)" == "rgb(0, 0, 255)" - 
[task 2018-04-18T14:36:18.895Z] 14:36:18     INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js | Element top border color should be set. - "rgb(0, 0, 255)" == "rgb(0, 0, 255)" - 
[task 2018-04-18T14:36:18.895Z] 14:36:18     INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js | Element bottom border color should be set. - "rgb(0, 0, 255)" == "rgb(0, 0, 255)" - 
[task 2018-04-18T14:36:18.900Z] 14:36:18     INFO - GECKO(3690) | --DOMWINDOW == 6 (0x7f5506bcf400) [pid = 3741] [serial = 2] [outer = (nil)] [url = about:blank]
[task 2018-04-18T14:36:18.901Z] 14:36:18     INFO - GECKO(3690) | --DOMWINDOW == 5 (0x7f5502e05400) [pid = 3741] [serial = 6] [outer = (nil)] [url = about:blank]
[task 2018-04-18T14:36:18.902Z] 14:36:18     INFO - Leaving test bound test_popup_styling
[task 2018-04-18T14:36:18.903Z] 14:36:18     INFO - GECKO(3690) | MEMORY STAT | vsize 2077MB | residentFast 301MB | heapAllocated 102MB
[task 2018-04-18T14:36:18.904Z] 14:36:18     INFO - TEST-OK | toolkit/components/extensions/test/browser/browser_ext_themes_arrowpanels.js | took 3055ms
[task 2018-04-18T14:36:18.905Z] 14:36:18     INFO - GECKO(3690) | ++DOCSHELL 0x7f221537b800 == 3 [pid = 3770] [id = {d60c4a74-7792-488f-a31b-5db5c4fea50d}]
[task 2018-04-18T14:36:18.906Z] 14:36:18     INFO - GECKO(3690) | ++DOMWINDOW == 6 (0x7f221df67c00) [pid = 3770] [serial = 6] [outer = (nil)]
[task 2018-04-18T14:36:18.927Z] 14:36:18     INFO - GECKO(3690) | ++DOMWINDOW == 7 (0x7f221df68800) [pid = 3770] [serial = 7] [outer = 0x7f221df67c00]
[task 2018-04-18T14:36:18.966Z] 14:36:18     INFO - checking window state
[task 2018-04-18T14:36:19.083Z] 14:36:19     INFO - TEST-START | toolkit/components/extensions/test/browser/browser_ext_themes_autocomplete_popup.js
[task 2018-04-18T14:36:19.462Z] 14:36:19     INFO - GECKO(3690) | [Parent 3690, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /builds/worker/workspace/build/src/netwerk/protocol/res/SubstitutingProtocolHandler.cpp, line 338
[task 2018-04-18T14:36:19.640Z] 14:36:19     INFO - GECKO(3690) | ++DOCSHELL 0x7f06e0323000 == 8 [pid = 3690] [id = {c0723d13-e8a6-499a-9e45-ff54694b1482}]
[task 2018-04-18T14:36:19.640Z] 14:36:19     INFO - GECKO(3690) | ++DOMWINDOW == 19 (0x7f06ecd3d400) [pid = 3690] [serial = 19] [outer = (nil)]
[task 2018-04-18T14:36:19.741Z] 14:36:19     INFO - GECKO(3690) | ++DOMWINDOW == 20 (0x7f06ecd42400) [pid = 3690] [serial = 20] [outer = 0x7f06ecd3d400]
[task 2018-04-18T14:36:20.045Z] 14:36:20     INFO - GECKO(3690) | ++DOMWINDOW == 21 (0x7f06ea1c7000) [pid = 3690] [serial = 21] [outer = 0x7f06ecd3d400]
[task 2018-04-18T14:36:20.264Z] 14:36:20     INFO - GECKO(3690) | --DOCSHELL 0x7f221ac96000 == 2 [pid = 3770] [id = {5612717b-d5e2-46b7-a716-229e3dd594b7}]
[task 2018-04-18T14:36:20.264Z] 14:36:20     INFO - GECKO(3690) | --DOCSHELL 0x7f221ae2c000 == 1 [pid = 3770] [id = {1414dce0-05bf-4895-8f01-4d42151ec843}]
[task 2018-04-18T14:36:21.252Z] 14:36:21     INFO - GECKO(3690) | --DOMWINDOW == 4 (0x7f5506bca000) [pid = 3741] [serial = 1] [outer = (nil)] [url = about:blank]
[task 2018-04-18T14:36:21.778Z] 14:36:21     INFO - GECKO(3690) | [Parent 3690, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /builds/worker/workspace/build/src/netwerk/protocol/res/SubstitutingProtocolHandler.cpp, line 338
[task 2018-04-18T14:36:22.703Z] 14:36:22     INFO - GECKO(3690) | MEMORY STAT | vsize 2100MB | residentFast 314MB | heapAllocated 104MB
[task 2018-04-18T14:36:22.703Z] 14:36:22     INFO - TEST-OK | toolkit/components/extensions/test/browser/browser_ext_themes_autocomplete_popup.js | took 3621ms
[task 2018-04-18T14:36:22.719Z] 14:36:22     INFO - GECKO(3690) | ++DOCSHELL 0x7f6a2d464000 == 2 [pid = 3834] [id = {f1c7a473-79eb-4da5-992f-6eae979c7cd2}]
[task 2018-04-18T14:36:22.720Z] 14:36:22     INFO - GECKO(3690) | ++DOMWINDOW == 3 (0x7f6a2d412000) [pid = 3834] [serial = 3] [outer = (nil)]
[task 2018-04-18T14:36:22.795Z] 14:36:22     INFO - GECKO(3690) | ++DOMWINDOW == 4 (0x7f6a2d413400) [pid = 3834] [serial = 4] [outer = 0x7f6a2d412000]
[task 2018-04-18T14:36:22.811Z] 14:36:22     INFO - checking window state
Flags: needinfo?(jaws)
Before spending a bunch of time figuring it out. The reason that happens is because the bindings are including XUL elements from layout that trigger the load of the full XUL stylesheet for content pages.

Can we avoid that please? Not only because it breaks style system invariants that we want (like "no sheets added from layout"), but also because that's a memory usage issue for every page that uses the datetimepicker.
Oh, I guess in this case it's the svg sheet... That's unfortunate :(
Flags: needinfo?(jaws)
The try push is green except for a crashtest that is failing (bug 1455005):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=481c050e4b9f224ab9cb7763d7c974b11eb83d40

Emilio, since this is caused by us using SVG here is there some way we can ease the crashtest? Does loading of the SVG stylesheet actually cause a crash here?
Flags: needinfo?(emilio)
No longer blocks: png-cleanup
dholbert, looks like we're getting this issue because I inlined the SVG within the anonymous content of the <input type="date">. Previously this was an external reference that was loaded via `url(chrome://global/skin/icons/input-clear.svg)`. I had to switch away from using `background-image` because `context-fill` isn't supported in content. This normally wouldn't be a problem but we have a crashtest that fails if <input type="date"> tries to load any UA sheets, and my patch causes it to load the SVG stylesheet (as described in bug 1455005 comment 4).

Would it be possible to allow context-fill and context-fill-opacity to work in anonymous nodes of content?
Flags: needinfo?(dholbert)
[Redirecting to jwatt, who implemented context-fill/context-fill-opacity]
Flags: needinfo?(dholbert) → needinfo?(jwatt)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #33)
> The try push is green except for a crashtest that is failing (bug 1455005):
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=481c050e4b9f224ab9cb7763d7c974b11eb83d40
> 
> Emilio, since this is caused by us using SVG here is there some way we can
> ease the crashtest? Does loading of the SVG stylesheet actually cause a
> crash here?

I'd rather avoid it, tbh, that doesn't change the fact that we're adding a stylesheet during layout.

The relevant bit that adds the sheet synchronously is:

  https://searchfox.org/mozilla-central/rev/59a9a86553e9bfd9277202748ff791fd9bc0713b/dom/svg/SVGSVGElement.cpp#528

Ideally we'd use Shadow DOM instead of XBL and this wouldn't be a problem since we wouldn't do DOM stuff from Layout.

I think we can work around it a bit if we ensure the SVG sheet is loaded when we know the input element is going to be a datetime input. Though it's a bit ugly. Not sure if the context-fill bits is easier, otherwise let me know and I can do that.
Flags: needinfo?(emilio)
Depends on: 1469176
I've put up a patch in bug 1469176 that should unblock this bug.

(In reply to Emilio Cobos Álvarez [:emilio] from comment #36)
> I think we can work around it a bit if we ensure the SVG sheet is loaded
> when we know the input element is going to be a datetime input. Though it's
> a bit ugly.

Yeah, that seemed unnecessarily ugly so I just went for making all documents load the SVG sheet. It shouldn't be a big deal.
Flags: needinfo?(jwatt)
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/01d18931ac33
DateTimePicker should support high contrast mode on Windows. r=dao
https://hg.mozilla.org/integration/autoland/rev/bb51c20029b8
Clean up some duplicated colors in the DateTimePicker. r=dao
https://hg.mozilla.org/mozilla-central/rev/01d18931ac33
https://hg.mozilla.org/mozilla-central/rev/bb51c20029b8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Regressions: 1641978
You need to log in before you can comment on or make changes to this bug.