Closed
Bug 1215856
Opened 9 years ago
Closed 8 years ago
":" should not be selected or copied in Title field
Categories
(Firefox :: Page Info Window, defect)
Tracking
()
VERIFIED
FIXED
Firefox 52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: yfdyh000, Unassigned)
References
Details
Attachments
(5 files, 2 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
I think people generally need to copy this field as an filename or anything, and this colon added to the results leads to trouble. For example, it is an illegal character for path in Windows.
I think there are several solutions:
1. Automatic clipping the results when hotkeys or context menus.
2. Automatically cancel its selection state when selected event, which may result in an inconsistent jump.
3. Avoid it is selected. I hope this can be done, and not by a single element, so as not to affect the field area.
p.s. It should also be allowed to be l10n.
I am new to open source community and want to solve this bug as my first bug.Can you please guide me as to how can I proceed.
https://developer.mozilla.org/en-US/docs/Introduction
The code:
https://hg.mozilla.org/mozilla-central/annotate/cdcd33fd6e39cd12feb5bb11951e1c981a04bd86/browser/base/content/pageinfo/pageInfo.js#l533
I look forward to helping you, if you need more info.
Assignee: nobody → pushkarpathak21
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 3•8 years ago
|
||
Since there has been no update on this bug for a long time, I thought of picking it up. What if there is no ':' at the first place?
Flags: needinfo?(yfdyh000)
Also good, but it would be better if there is a sense of depth or shading for the sub-information.
Assignee: pushkarpathak21 → nobody
Severity: normal → minor
Status: ASSIGNED → NEW
Flags: needinfo?(yfdyh000)
Keywords: uiwanted
Comment 5•8 years ago
|
||
Well the title is already bolder than the sub-information.Please let me know what else do you suggest in order to add a sense of depth to the sub-information while omitting the ':' from the tite?
Flags: needinfo?(yfdyh000)
Like indents and triangle arrow, or frame border.
Flags: needinfo?(yfdyh000)
Comment 7•8 years ago
|
||
Added border around the subsection
(In reply to Deepjyoti Mondal from comment #7)
> Created attachment 8776578 [details]
> Screenshot from 2016-08-01 15-15-27.png
>
> Added border around the subsection
It is not what I think the frame border (like webpage frame).
I'm not a reviewer.
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Submitting_the_patch
Whiteboard: [good first bug]
Attachment #8776579 -
Flags: review?(yfdyh000)
Comment 10•8 years ago
|
||
Sorry for bothering you with review.
It seems I confused frame border with ordinary css border. Can you please provide me with an example of frame border so that I can continue with the patch.
Flags: needinfo?(yfdyh000)
Reporter | ||
Comment 11•8 years ago
|
||
Note that my ideas is not an order.
I like the UI (sub-information) like screenshot.
A frame border like webpage's border in Microsoft IE, but it may not be common in Firefox.
Flags: needinfo?(yfdyh000)
Comment 12•8 years ago
|
||
I modified the appearance of the subsection below the title field and the title field itself to make it look like the screen shot in the previous post.
Attachment #8776579 -
Attachment is obsolete: true
Attachment #8777942 -
Flags: review?(dtownsend)
Comment 13•8 years ago
|
||
Comment on attachment 8777942 [details] [diff] [review]
bug1215856v2.patch
Review of attachment 8777942 [details] [diff] [review]:
-----------------------------------------------------------------
I'm not sure that the proposal is the right choice here. Stephen, can you weigh in or find someone to weigh in on what that section of the page info dialog should look like?
::: browser/base/content/pageinfo/pageInfo.js
@@ +533,4 @@
> function makeGeneralTab(metaViewRows, docInfo)
> {
> var title = (docInfo.title) ? gBundle.getFormattedString("pageTitle", [docInfo.title]) : gBundle.getString("noPageTitle");
> + document.getElementById("titletext").label = title.slice(0,title.length-1);
The correct change would be to edit the l10n strings to remove the :.
Attachment #8777942 -
Flags: review?(dtownsend) → ui-review?(shorlander)
Comment 14•8 years ago
|
||
Can you please point me out where can I find the l10n strings?
Flags: needinfo?(dtownsend)
Comment 15•8 years ago
|
||
Here: https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/pageInfo.properties#8
Changing strings means changing the string key too but let's wait on a UX response first.
Flags: needinfo?(dtownsend)
Comment 16•8 years ago
|
||
Okay
Comment 17•8 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #13)
> I'm not sure that the proposal is the right choice here. Stephen, can you
> weigh in or find someone to weigh in on what that section of the page info
> dialog should look like?
We could just make the title consistent with the other information. E.g.:
Title: Some title goes here
Address: http://address.address
Type: text/html
etc.
Updated•8 years ago
|
Attachment #8777942 -
Flags: ui-review?(shorlander)
Comment 18•8 years ago
|
||
Should the title remain bold as it is now or should it be normal like the other information.
Flags: needinfo?(shorlander)
Comment 19•8 years ago
|
||
This is a test patch.I have tried to make the title consistent with the other fields.
Attachment #8792659 -
Flags: review?(dtownsend)
Comment 20•8 years ago
|
||
Comment on attachment 8792659 [details] [diff] [review]
bug1215856_v2.patch
Review of attachment 8792659 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/locales/en-US/chrome/browser/pageInfo.properties
@@ +6,4 @@
> pageInfo.frame.title=Frame Info - %S
>
> noPageTitle=Untitled Page:
> +pageTitle=%S
This is unnecessary now isn't it?
Attachment #8792659 -
Flags: review?(dtownsend)
Comment 21•8 years ago
|
||
modified my previous patch.
Attachment #8792659 -
Attachment is obsolete: true
Attachment #8792949 -
Flags: review?(dtownsend)
Comment 22•8 years ago
|
||
Comment on attachment 8792949 [details] [diff] [review]
bug1215856_v2.patch
Review of attachment 8792949 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, thanks for the patch.
Attachment #8792949 -
Flags: review?(dtownsend) → review+
Comment 23•8 years ago
|
||
(In reply to Deepjyoti Mondal from comment #18)
> Should the title remain bold as it is now or should it be normal like the
> other information.
Since it's just another piece of site information I would make it the same normal weight as the rest.
Flags: needinfo?(shorlander)
Comment 24•8 years ago
|
||
Most of the issues mentioned in bug 1308737 are solved by the previous patch (8792949) except the Title Case issue. Since most of the work has already been done in this bug, will it not be better to extend the previous patch and address the Title Case issue here itself ?
Flags: needinfo?(dtownsend)
Comment 25•8 years ago
|
||
(In reply to Deepjyoti Mondal from comment #24)
> Most of the issues mentioned in bug 1308737 are solved by the previous patch
> (8792949) except the Title Case issue. Since most of the work has already
> been done in this bug, will it not be better to extend the previous patch
> and address the Title Case issue here itself ?
It's unclear whether bug 1308737 is a workable bug right now, it would require designs to move forward. We should just land this patch as it is ready to go.
Flags: needinfo?(dtownsend)
Keywords: uiwanted → checkin-needed
Comment 26•8 years ago
|
||
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2dd4ac14010d
"":" should not be selected or copied in Title field". r=dtownsend
Keywords: checkin-needed
Comment 27•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Comment 28•8 years ago
|
||
I have reproduced this bug with Nightly 44.0a1 (2015-10-17) on ubuntu 16.10,64 Bit!
This bug's fix is verified with latest Beta !
Build ID : 20170216105119
User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
[testday-20170217]
You need to log in
before you can comment on or make changes to this bug.
Description
•