Closed
Bug 1488807
Opened 6 years ago
Closed 6 years ago
[de-xbl] Remove dialogheader binding.
Categories
(Thunderbird :: Account Manager, task)
Thunderbird
Account Manager
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 64.0
People
(Reporter: arshad, Assigned: arshad)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
arshad
:
review+
|
Details | Diff | Splinter Review |
dialogheader has description attribute[1] which is never used except one time in /suite[2]. I am thinking of converting it into custom element. Should I add dialogheader-description or not into the new element?
[1]: https://searchfox.org/comm-central/source/common/bindings/generalBindings.xml#45
[2]: https://searchfox.org/comm-central/search?q=(%3Cdialogheader+.%2B+description)&case=false®exp=true&path=
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → arshdkhn1
Blocks: tb-war-on-xbl
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(mkmelin+mozilla)
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #9006669 -
Flags: review?(mkmelin+mozilla)
Comment 2•6 years ago
|
||
Looking at the few isolated usages of dialogheader, it seems to me it would be preferable to get rid of this element by open coding it. It woudl be replaced by a label (and perhaps a containing box?) easily. There's no functionality, only some css.
That said, don't let it stop the work you're doing to get the "infrastructure" set up (customElements.js file) as that would be useful elsewhere.
So to answer what to do with the description: nothing.
Makes sense?
Flags: needinfo?(mkmelin+mozilla)
Comment 3•6 years ago
|
||
Comment on attachment 9006669 [details] [diff] [review]
dialogheader.patch
For WIP patches, ask for feedback only.
When you think the patch is ready, then ask for review.
Attachment #9006669 -
Attachment is obsolete: true
Attachment #9006669 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 4•6 years ago
|
||
Assignee | ||
Comment 5•6 years ago
|
||
Attachment #9006911 -
Attachment is obsolete: true
Assignee | ||
Comment 6•6 years ago
|
||
Attachment #9006922 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #9006932 -
Flags: review?(mkmelin+mozilla)
Comment 7•6 years ago
|
||
Comment on attachment 9006932 [details] [diff] [review]
dialogheader.patch
Review of attachment 9006932 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me. r=mkmelin
::: mailnews/base/prefs/content/am-main.js
@@ +15,4 @@
> else
> titleValue = defaultTitle;
>
> + title.setAttribute("value",titleValue);
let's add the missing space after comma while we're here
Attachment #9006932 -
Flags: review?(mkmelin+mozilla) → review+
Is there a second patch / follow-up bug and patch coming for the suite/ parts?
Flags: needinfo?(arshdkhn1)
Assignee | ||
Comment 9•6 years ago
|
||
Added the white space.
Attachment #9006932 -
Attachment is obsolete: true
Flags: needinfo?(arshdkhn1)
Assignee | ||
Comment 10•6 years ago
|
||
(In reply to Ian Neal from comment #8)
> Is there a second patch / follow-up bug and patch coming for the suite/
> parts?
I am not sure whether I have to create a second patch / follow-up bug for suite. I think Magnus will be right person to ask this.
Comment 11•6 years ago
|
||
I don't think we should fix up suite here. But we can clone the bug to suite and let seamonkey people handle suite/ parts there.
Assignee | ||
Comment 12•6 years ago
|
||
Attachment #9007411 -
Attachment is obsolete: true
Attachment #9014328 -
Flags: review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 13•6 years ago
|
||
You guys got a try run? Here we go:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e9802ed9dc90642b6f734c2c990d44c8734d57f3
Assignee | ||
Comment 14•6 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #13)
> You guys got a try run? Here we go:
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=e9802ed9dc90642b6f734c2c990d44c8734d57f3
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=055078b0ca59fac37b8858df54d68cf34065101f
Sorry i forgot to comment it.
Comment 15•6 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/5e8a03122a95
Removing dialogheader binding; r=mkmelin
Updated•6 years ago
|
Target Milestone: --- → Thunderbird 64.0
Updated•5 years ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•