Note: There are a few cases of duplicates in user autocompletion which are being worked on.

":" should not be selected or copied in Title field

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Page Info Window
--
minor
VERIFIED FIXED
2 years ago
5 months ago

People

(Reporter: YF (Yang), Unassigned)

Tracking

42 Branch
Firefox 52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
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.

Comment 1

2 years ago
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.
(Reporter)

Comment 2

2 years ago
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

a year ago
Created attachment 8776285 [details]
Screenshot from 2016-07-30 21-12-28.png

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)
(Reporter)

Comment 4

a year ago
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

a year 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)
(Reporter)

Comment 6

a year ago
Like indents and triangle arrow, or frame border.
Flags: needinfo?(yfdyh000)

Comment 7

a year ago
Created attachment 8776578 [details]
Screenshot from 2016-08-01 15-15-27.png

Added border around the subsection

Comment 8

a year ago
Created attachment 8776579 [details] [diff] [review]
Bug1215856.patch

Patch for the above.
Attachment #8776579 - Flags: review?(yfdyh000)
(Reporter)

Comment 9

a year ago
(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]
(Reporter)

Updated

a year ago
Attachment #8776579 - Flags: review?(yfdyh000)

Comment 10

a year 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

a year ago
Created attachment 8776636 [details]
Screenshot from 2016-07-30 21-12-28 - Modified.png

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

a year ago
Created attachment 8777942 [details] [diff] [review]
bug1215856v2.patch

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 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

a year ago
Can you please point me out where can I find the l10n strings?
Flags: needinfo?(dtownsend)
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

a year ago
Okay
(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.
Attachment #8777942 - Flags: ui-review?(shorlander)

Comment 18

10 months ago
Should the title remain bold as it is now or should it be normal like the other information.
Flags: needinfo?(shorlander)

Comment 19

10 months ago
Created attachment 8792659 [details] [diff] [review]
bug1215856_v2.patch

This is a test patch.I have tried to make the title consistent with the other fields.
Attachment #8792659 - Flags: review?(dtownsend)
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

10 months ago
Created attachment 8792949 [details] [diff] [review]
bug1215856_v2.patch

modified my previous patch.
Attachment #8792659 - Attachment is obsolete: true
Attachment #8792949 - Flags: review?(dtownsend)
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+
(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)
(Reporter)

Updated

9 months ago
See Also: → bug 1308737

Comment 24

9 months 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)
(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

9 months 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
https://hg.mozilla.org/mozilla-central/rev/2dd4ac14010d
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52

Comment 28

5 months 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]
(Reporter)

Updated

5 months ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.