746312 - remove unused id="gray_horizontal_splitter" on splitters

Status: RESOLVED WONTFIX

(Thunderbird :: Theme, defect)

Description

[:aceman]

Bug 205039 comment 35 noticed there are splitters with id="gray_horizontal_splitter" but there is no css and no code attached to these IDs (see the mxr search). There are 4 occurrences of this, 6 when bug 205039 lands.

I'd like to ask the theme experts if there is any use of these IDs or they can be removed. There are tons of splitter in TB/Seamonkey that do not have this ID.

Comment 1

[Richard Marti (:Paenglab)]

I would say you can remove this because it isn't specially used. Also when I think of ID's which should be unique and this ID exists 4 to 6 times then I'm for removing this.

Comment 2

[Magnus Melin [:mkmelin]]

Well, it's in different windows so same id is ok. Why would you remove it? In general it's good to have id's for most elements - it helps extensions and theming a lot. It's not the best of names of course...

Comment 3

[Stefan [:stefanh]]

Actually, in bug 205039, my main concern was adding another "gray_horizontal_splitter" id. Having an id can be good, but I don't think there's any point adding one when we're not using it. But if an id should be added, I don't think you should add one with that kind of name.

Comment 4

[Stefan [:stefanh]]

(In reply to Stefan [:stefanh] from comment #3)
> But if an id should be
> added, I don't think you should add one with that kind of name.

Never mind - that's just my opinion.

Comment 5

[:aceman]

Adding the id with the existing name in bug 205039 is for consistency. If any theme is setting style on the ID due to the search dialog, it may also want to set the same style on the filter editor dialog.

I think everybody objects to the name. It should at least not contain the color hardcoded :)

The point of this bug is to decide whether to remove all the IDs, or rename them to something better.

Comment 6

[Magnus Melin [:mkmelin]]

There is value in having ids for stuff even while it's not used in "core" - some extension might want to hook in and add something there. People have the strangest needs :)

Comment 7

[:aceman]

So what about "dialog_splitter_horizontal" or "search_splitter_horizontal" or "search_splitter" ? It seems it is only used in search dialogs and I have not seen any vertical one.

Comment 8

[:aceman]

Ping? Is there any conclusion here?

Comment 9

[:aceman]

According to https://developer.mozilla.org/en-US/docs/XUL/Attribute/persist the id is needed for the "persist" attribute to work properly :) So that solves the question of removing the ID.

For the name change, if we change the ID it will probably mean the new TB will not pickup the saved values of the splitter for existing users and will reset the splitter state. That also seems like a problem... However I am not sure we actually store any state for it, except the splitter in ABSearchDialog.xul which can be collapsed.

Comment 10

[Josiah Bruner [:jsbruner]]

Actually, many developers may use this so I think we should keep this ID.