Move <filefield> to toolkit/content/widgets

RESOLVED FIXED

Status

()

Toolkit
Preferences
--
enhancement
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

10 years ago
In bug 443410 Gavin mentioned that he wanted all the CSS for filefield centralised and I would prefer if we centralised filefield itself first.
(Assignee)

Comment 1

10 years ago
Created attachment 339251 [details] [diff] [review]
Proposed patch
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #339251 - Flags: review?(enndeakin)

Comment 2

10 years ago
Comment on attachment 339251 [details] [diff] [review]
Proposed patch

Is this supposed to be a filepicker widget? It would be great to actually make it one with a means of opening a file picker as well.

>-*+ content/global/bindings/findbar.xml         (widgets/findbar.xml)

This doesn't seem intentional.

>-  <binding id="fileField" extends="chrome://global/content/bindings/general.xml#control-item">
>+  <binding id="filefield" extends="chrome://global/content/bindings/general.xml#control-item">

Shouldn't this just extend from basetext? The extra value property doesn't seem to do anything.

>       <xul:stringbundle anonid="bundle" src="chrome://mozapps/locale/preferences/preferences.properties"/>

We should move this too.
(Assignee)

Comment 3

10 years ago
Created attachment 339269 [details] [diff] [review]
Updated for comments
Attachment #339251 - Attachment is obsolete: true
Attachment #339269 - Flags: review?(enndeakin)
Attachment #339251 - Flags: review?(enndeakin)

Comment 4

10 years ago
Do we want the other password related properties there too?

What about this comment:

> Shouldn't this just extend from basetext? The extra value property
> doesn't seem to do anything.
(Assignee)

Comment 5

10 years ago
(In reply to comment #4)
> Do we want the other password related properties there too?
Sorry, over-enthusiastic interpretation of your comment.

> What about this comment:
> > Shouldn't this just extend from basetext? The extra value property
> > doesn't seem to do anything.
I'm sure I had it in an edit... good thing I need a new patch ;-)
(Assignee)

Comment 6

10 years ago
Created attachment 339332 [details] [diff] [review]
Fixed patch

Third time lucky?
Attachment #339269 - Attachment is obsolete: true
Attachment #339332 - Flags: review?(enndeakin)
Attachment #339269 - Flags: review?(enndeakin)

Comment 7

10 years ago
Comment on attachment 339332 [details] [diff] [review]
Fixed patch

Looks good. I'm not sure about calling it 'filefield' though, as it's inconsistent with other elements. It would probably more naturally be called 'filebox' although the 'box' name that seems overused. <filepicker> would be a good name if we added a 'Choose' button.
Attachment #339332 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 8

10 years ago
Pushed changeset 4f1e7711b92f to mozilla-central. I remembered the rename, but forgot the copy, but it was too late, so f02d0b4fb198 is just an add...
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Summary: Move <filefield> to toolkit/content/widgets? → Move <filefield> to toolkit/content/widgets

Updated

10 years ago
Blocks: 459372

Comment 9

10 years ago
The .css part of filefield should still be moved from /browser/skin/preferences/... to /mozapps/skin/preferences/preferences.css or even to /global/skin/preferences.css?

Created bug 459372 for this.
You need to log in before you can comment on or make changes to this bug.