Closed
Bug 1341376
Opened 7 years ago
Closed 7 years ago
HTMLInputElement::ValueAsDateEnabled and HTMLInputElement::SetFilesOrDirectories should use BoolVarCache
Categories
(Core :: DOM: Core & HTML, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: smaug, Assigned: jessica)
References
Details
Attachments
(1 file, 1 obsolete file)
16.31 KB,
patch
|
jessica
:
review+
|
Details | Diff | Splinter Review |
GetBool shows up in some profiles.
Comment 1•7 years ago
|
||
Hsin-Yi, anyone on your team have time to take this?
Flags: needinfo?(htsai)
Priority: -- → P1
Comment 2•7 years ago
|
||
Same with Exception::Initialize().
Updated•7 years ago
|
Assignee: nobody → jjong
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to :Ehsan Akhgari from comment #2) > Same with Exception::Initialize(). At a first glance, didn't find calls to Preferences::GetBool() in Exception::Initializa() [1] though. [1] https://searchfox.org/mozilla-central/rev/39e4b25a076c59d2e7820297d62319f167871449/dom/base/DOMException.cpp#397-421
Assignee | ||
Comment 6•7 years ago
|
||
Comment on attachment 8839832 [details] [diff] [review] patch, v1. Will file another bug to do some clean-up/merging on datetime and experimental_forms preferences.
Attachment #8839832 -
Flags: review?(bugs)
Reporter | ||
Comment 7•7 years ago
|
||
yeah, I can't see any Get* pref method calls in Exception either. Ehsan, perhaps you meant some other class? But please file another bug for that.
Flags: needinfo?(ehsan)
Comment 8•7 years ago
|
||
(In reply to Olli Pettay [:smaug] (pto-ish for couple of days) from comment #7) > yeah, I can't see any Get* pref method calls in Exception either. > Ehsan, perhaps you meant some other class? But please file another bug for > that. The profile here clearly shows the call: https://perfht.ml/2l8Rq1f. There is probably some inlining involved. Bug 1341716.
Flags: needinfo?(ehsan)
Reporter | ||
Comment 9•7 years ago
|
||
Comment on attachment 8839832 [details] [diff] [review] patch, v1. >+HTMLInputElement::IsExperimentalFormsEnabled() { Nit, { to its own line > if (success) { > newType = aResult.GetEnumValue(); > if ((IsExperimentalMobileType(newType) && >- !Preferences::GetBool("dom.experimental_forms", false)) || >+ !IsExperimentalFormsEnabled()) || > (newType == NS_FORM_INPUT_NUMBER && > !Preferences::GetBool("dom.forms.number", false)) || > (newType == NS_FORM_INPUT_COLOR && > !Preferences::GetBool("dom.forms.color", false)) || Ah, want to file a separate bug for number and color?
Attachment #8839832 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Olli Pettay [:smaug] (pto-ish for couple of days) from comment #9) > Comment on attachment 8839832 [details] [diff] [review] > patch, v1. > > >+HTMLInputElement::IsExperimentalFormsEnabled() { > Nit, { to its own line > > > > if (success) { > > newType = aResult.GetEnumValue(); > > if ((IsExperimentalMobileType(newType) && > >- !Preferences::GetBool("dom.experimental_forms", false)) || > >+ !IsExperimentalFormsEnabled()) || > > (newType == NS_FORM_INPUT_NUMBER && > > !Preferences::GetBool("dom.forms.number", false)) || > > (newType == NS_FORM_INPUT_COLOR && > > !Preferences::GetBool("dom.forms.color", false)) || > Ah, want to file a separate bug for number and color? Hmm, I think I'll do it in this bug, will upload a new patch for this.
Assignee | ||
Comment 11•7 years ago
|
||
Use BoolVarCache to get dom.forms.color and dom.forms.number prefs as well, carrying r+.
Attachment #8839832 -
Attachment is obsolete: true
Attachment #8840323 -
Flags: review+
Reporter | ||
Comment 12•7 years ago
|
||
Thanks
Assignee | ||
Comment 13•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=297445fa0fac1253efb78fcd4f15af6779dccb70&group_state=expanded
Keywords: checkin-needed
Comment 14•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/212db4739778 Use BoolVarCache to cache preferences in HTMLInputElement. r=smaug
Keywords: checkin-needed
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/212db4739778
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•