Create HUD style for name field in edit bookmark panel when it is a droppable menulist

RESOLVED WONTFIX

Status

()

Firefox
Theme
RESOLVED WONTFIX
8 years ago
5 years ago

People

(Reporter: Margaret, Assigned: Margaret)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

8 years ago
Created attachment 489989 [details] [diff] [review]
patch

We're currently using the native styles for the name field when it is a droppable menulist, which looks bad. Here's a patch that fixes it.

(This patch is built on top of the patch for bug 597557)
Attachment #489989 - Flags: review?(dao)
(Assignee)

Updated

8 years ago
Assignee: nobody → margaret.leibovic
(Assignee)

Comment 1

8 years ago
Created attachment 489990 [details]
before and after screenshots
Comment on attachment 489989 [details] [diff] [review]
patch

Hm, that's a bunch of code that would hardly ever be used (since microsummaries are more or less dead (bug 611676)), which makes this both regression-prone and probably not worth taking...
(In reply to comment #2)
> Hm, that's a bunch of code that would hardly ever be used (since microsummaries
> are more or less dead (bug 611676))

Err, bug 524091.
while I suppose microsummaries generators will be dropped pretty soon, I also guess support for existing microsummaries on pages will survive with some resize (thus their visualization). Also bugzilla has microsummaries.

I don't have strong feelings on the style, so I won't complain in both cases, but the screenshot is cool and gives much less sense of unpolished.
(In reply to comment #4)
> while I suppose microsummaries generators will be dropped pretty soon, I also
> guess support for existing microsummaries on pages will survive with some
> resize (thus their visualization). Also bugzilla has microsummaries.

This belongs in bug 524091, but I think we should drop the whole thing if basically nobody wants to use it. I don't think Mozilla projects count...
(Assignee)

Comment 6

8 years ago
I don't care too much either way. I was just thinking that we should make the feature look correct, as long as it's included in the product.
"unpolished", "look correct" etc. only apply if someone actually sees it... which is mostly not the case here, as far as I can tell. Even if we keep the feature for the time being (mostly for the lack of somebody committing to removing it, I guess), we shouldn't be taking arbitrary code to polish or improve it. This would be different if the patch was just setting a missing CSS class or something. As it is, I don't think it pays off.

Updated

8 years ago
Attachment #489989 - Flags: review?(dao)
Your "nobody sees it" point, hurts against the fact this is in our primary ui for bookmarking. Just bookmark a bugzilla search and you'll see it.

And we are not keeping microsummaries just because nobody has time to remove them, but because that requires a discussion regarding the direction we want to take. I think in the end we will kill most of them, but not their visualization thus this bug is valid.

The patch is pretty much contained and I don't see the regression-prone stuff here since these styles apply to just 1 element, I'd agree if it would apply to the full panel or multiple elements.
Thus I don't see a reason to drop this work. I'll ask a ui-r to Mike to get a decision.
Comment on attachment 489990 [details]
before and after screenshots

Mike, could you please give us an evaluation if you think it is worth to take this change for style coherence in the panel?
Attachment #489990 - Flags: ui-review?(beltzner)
(In reply to comment #8)
> Your "nobody sees it" point, hurts against the fact this is in our primary ui
> for bookmarking. Just bookmark a bugzilla search and you'll see it.

For hundreds of millions pages out there, it's not in our primary UI.

> And we are not keeping microsummaries just because nobody has time to remove
> them, but because that requires a discussion regarding the direction we want to
> take.

Ok. I'm not prepared to take this patch until that discussion has been had.

> The patch is pretty much contained and I don't see the regression-prone stuff
> here since these styles apply to just 1 element, I'd agree if it would apply to
> the full panel or multiple elements.

I didn't mean that the patch itself could cause regressions. I meant that it's unlikely that somebody would notice it if the styling broke, e.g. when somebody touches the panel style again.

Updated

8 years ago
Attachment #489989 - Flags: review-
Fwiw, not implementing fixes because somebody in future could break them, is a weak argument.
FX4 will have microsummaries, FX5 could be different, but we should base our current decisions on the product we are releasing imo.
I'm fine if we don't want to fix this style, but I'd like an acceptable argument to do so.
(In reply to comment #11)
> I'm fine if we don't want to fix this style, but I'd like an acceptable
> argument to do so.

The onus lies on the code wanting to get in, it needs to prove worthy. The fact that we still ship the feature isn't necessarily sufficient, e.g. if the feature isn't used despite being shipped.

Let me use this opportunity to mention that this bug goes back to Firefox 3.0, I think, but nobody seems to have cared... except for Margaret when she worked on the code!
The fact this is not a regression is a good enough argument to me, thanks. I think this now has enough info for a driver to take a decision.

Comment 14

7 years ago
Dão, you really **** me off.
My pleasure!
(Assignee)

Comment 16

7 years ago
We won't fix this because Marco is fixing bug 524091.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WONTFIX

Updated

6 years ago
Attachment #489990 - Flags: ui-review?(mbeltzner)
You need to log in before you can comment on or make changes to this bug.