1100139 - Firefox bookmark properties dialog should have a flexible description text field

Status: RESOLVED WONTFIX

(Firefox :: Bookmarks & History, defect)

Description

[Nicolas Mandil (:NicolasWeb)]

Attachment: Properties-expanded-size.png

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141013200257

Steps to reproduce:

1. open Bookmark lateral panel by ctrl+B
2. right click on a bookmark and open Properties Dialog
3. Expand the dialog


Actual results:

Description text field have a fixed high size : it keeps it original size


Expected results:

Description text field shoud have a min and relative size : it should expand to show as much text as the dialog box size allow

Comment 1

[Nicolas Mandil (:NicolasWeb)]

You can reproduce it too with hte following first step (that is more linked to the newer Bookmark UI) :
1. Open the bookmark menu in the main tool bar

I use the description field to add a summary to important bookmarks.

Comment 2

[Mike Hoye [:mhoye]]

Hi, Nicolas -

This looks like a good starter bug - can you track down a DXR link to show a contributor roughly where a fix would go, and add it to the bug?

Comment 3

[Nicolas Mandil (:NicolasWeb)]

Hi Mike,

I think that the file that need a fix is : http://dxr.mozilla.org/mozilla-central/source/browser/components/places/content/editBookmarkOverlay.xul

or the corresponding css file ;-)

Comment 4

[Nicolas Mandil (:NicolasWeb)]

Mike, may I mail you my DXR feedback, to tell you the issues I went thought (and better understand) please ?
(I read again the Mozilla Bugzilla Etiquette)
(Don't mail me at my Bugzilla email address, it is too messy : I don't check at the time)

Comment 5

[Vincent Lequertier]

Working on it as my first contribution :-)

Comment 6

[Mike Hoye [:mhoye]]

Hi, Nicolas - Yes, please.

Comment 7

[Vincent Lequertier]

Attachment: make the description textbox flexible

The patch does two things, it makes the text box flexible, and adds a minimal height.

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 8531633 [details] [diff] [review]
make the description textbox flexible

Review of attachment 8531633 [details] [diff] [review]:
-----------------------------------------------------------------

I'm forwarding this to Mano that is working on that dialog currently

Comment 9

[Vincent Lequertier]

Can someone review the patch please ?

Comment 10

[Mike Hoye [:mhoye]]

Thanks for the nudge, Sky. Mano, are you the right person for this?

Comment 11

[Marco Bonardo [:mak]]

Comment on attachment 8531633 [details] [diff] [review]
make the description textbox flexible

taking back

Comment 12

[Marco Bonardo [:mak]]

Comment on attachment 8531633 [details] [diff] [review]
make the description textbox flexible

Review of attachment 8531633 [details] [diff] [review]:
-----------------------------------------------------------------

I have a doubt here, the dialog should not be resizable, so I'm not totally sure making the Description field flex would really help anyone.
The screenshot you posted is strange cause it appears somehow you were able to resize the dialog. Could you please check if you see that empty space also in a new profile?

Notice I'm not saying we don't want to make the description field bigger, it's just that making that properly (by allowing to resize the dialog) could require more changes. We'd need to make the dialog resizeable, pick good min/max width and height and persist the last size. We should also remove existing hacks that allow the dialog do resize only in certain cases (and I think we don't even hit any of those anymore).

Moreover, description is a minor property we don't plan to invest on for now, making it so prominent could not be the best choice (it is not even searchable, it's usage is very limited).

This change by itself would not hurt, but it will likely be pointless without an overhaul of the dialog.

::: browser/components/places/content/editBookmarkOverlay.xul
@@ +168,5 @@
>                   accesskey="&editBookmarkOverlay.description.accesskey;"
>                   control="editBMPanel_descriptionField"/>
>            <textbox id="editBMPanel_descriptionField"
> +		   multiline="true"
> +		   flex="1"/>

I think your editor inserted some tabs here, we only use spaces for indentation.


The other problem is that currently we have another element that flexes vertically that is editBMPanel_folderTree, with this change one would need to enlarge the dialog double the previous amount to be able to see enough folders. I think the folderTree should flex=2, while description should flex=1

::: browser/themes/linux/places/editBookmarkOverlay.css
@@ +54,5 @@
>  }
>  
> +#editBMPanel_descriptionRow {
> +  min-height: 4em;
> +}

This sounds like something that would be needed for all themes, not just linux.
I'd also like if the min-height would be about the same as the default height, not more than that, I suspect something closer to 3em?

Comment 13

[Nicolas Mandil (:NicolasWeb)]

Attachment: Dialog with a fresh new profile.png

> Notice I'm not saying we don't want to make the description field bigger
:-)

> I have a doubt here, the dialog should not be resizable, so I'm not totally 
> sure making the Description field flex would really help anyone.
Having a Description field with 2.5 lines visible is kind of not good UX... especially if the user is not able to expand the field (by resizing the dialog for exemple).
I think that's a feature, not a bug.
In the Library the issue is the same (description field not resizeable), but the context different (item list resizable). Giving a better UX for that case could be nice too ;-)

> Could you please check if you see that empty space also in a new profile?
The issue is still here.

> Moreover, description is a minor property we don't plan to invest on for now, 
> making it so prominent could not be the best choice (it is not even searchable, 
> it's usage is very limited).
I understand. I would prefer to have a tool to annotate in-content my bookmarked pages.
In the waiting ;-) allowing it to be a bit bigger for those who use it will be highly appreciated.

Comment 14

[Nicolas Mandil (:NicolasWeb)]

I just checked with a new profile on Windows 7 : I'm not able to resize the dialog (happy GNU/Linux user I am...)

Comment 15

[Vincent Lequertier]

I don't fully understand what you are saying, I'm not good in English, but this is what I've done :

* Removed the unnecessary tabs, and increased the "flex" value of editBMPanel_folderTree
* Moved the changes from browser/themes/linux/places/editBookmarkOverlay.css to browser/base/content/browser.css and set min-height to 3em.

The dialog seems to be resizeable on linux only.

Comment 16

[Nicolas Mandil (:NicolasWeb)]

Mike can you help at this point ?

Comment 17

[Mike Hoye [:mhoye]]

Ok, it looks like Mano is working on other stuff right now, I'm calling in reinforcements.

Comment 18

[Mike Hoye [:mhoye]]

Looks like Mak is the module owner here. Marco, can we get an executive decision on this bug?

Comment 19

[Nicolas Mandil (:NicolasWeb)]

Precisions on comment #13 :
> Having a Description field with 2.5 lines visible is kind of not good UX... especially if the user is
> not able to expand the field (by resizing the dialog for exemple).

It's hard to use as the scrolling scale is 3 lines : more than the visible ones.

Comment 20

[Marco Bonardo [:mak]]

I hate to say "no", but the change suggested can be more expensive than expected.

The problem, as I said, is that:
1. on most systems (ideally all of them) the dialog is not resizable. The fact it's resizable is a bug (that I think exists only on the Linux version).
2. Making the dialog resizable for everyone would be nice, if its contents would be properly "liquid" and adapt to the size. Unfortunately we are not there yet.
3. This UI really needs a revamp, and I don't think it will be dialogs still.
4. Making a field we consider less important, very prominent, is not what we want.

Due to the above, while the suggested change is not extremely complicated, it would just be built on top of an unintended bug. Plus it will add a new behavior to a bogus and not coherent dialog, that could have unexpected consequences and maintenance costs in futre.

What I'd probably accept as a change, if you think the current description field is too small, is a slight increase in its minimum number of rows, that is, we could add rows="4" to its definition.
But please, if you think this might be useful, file a separate bug and report it here, since it's conceptually different from this bug request.

Comment 21

[Nicolas Mandil (:NicolasWeb)]

Earing "no" is never pleasant, but when argue it can be understood.
I hope that this UI can be revamp with the come of a nicer feature (bookmark parts of pages).
As the actual issue about scrolling scale VS input size can have an intermediate solution, I filled bug 1149198