622405 - Container of browser.xul identity-popup-more-info-button (an hbox) is missing ID

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Soumya Deb [:Debloper]]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b8) Gecko/20100101 Firefox/4.0b8
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b8) Gecko/20100101 Firefox/4.0b8

In the site identity-popup, the last element ("More Information..." button, id="identity-popup-more-info-button") is contained within an hbox without any id.

If for any reason, a button is required to be added in that hbox set, it takes relatively complex approach to get it done. Adding an id to the hbox can fix it.

Even at the time of this bug-reporting, the latest revision of browser.xul has this problem - http://hg.mozilla.org/mozilla-central/file/a05e91710adb/browser/base/content/browser.xul

Reproducible: Always

Comment 1

[Soumya Deb [:Debloper]]

Attachment: Screenshot of the identity-popup

Comment 2

[Soumya Deb [:Debloper]]

Attachment: Solution code as text-file

Comment 3

[Ed Morley [:emorley]]

Confirmed with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20110101 Firefox/4.0b9pre ID:20110101030401

Comment 4

[Soumya Deb [:Debloper]]

Attachment: Added id="identity-popup-more-info" to the hbox to fix

Not sure if I've added the reviewer string right...

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 500696 [details] [diff] [review]
Added id="identity-popup-more-info" to the hbox to fix

Please see https://developer.mozilla.org/en/Creating_a_patch and https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_diff_and_patch_files.3F

Comment 6

[Soumya Deb [:Debloper]]

Attachment: Diff created after adding id="identity-popup-more-info" to the hbox to fix

I guess, dao is available for this, & hope I've done the right thing!

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 500752 [details] [diff] [review]
Diff created after adding id="identity-popup-more-info" to the hbox to fix

Your editor replaced all \n line endings with \r\n ones, resulting in an enormous patch. Please fix ;)

Comment 8

[Soumya Deb [:Debloper]]

Attachment: Added id="identity-popup-more-info" to the hbox to fix

Thanks to irc:Kwierso for helping me out.
I'm pretty sure, I got it right this time.

@Dao: thanks for having patients with me; really appreciate it.

Comment 9

[Dão Gottwald [:dao]]

I'd suggest "identity-popup-button-container" as the id, given this rationale:

> If for any reason, a button is required to be added in that hbox set, it takes
> relatively complex approach to get it done. Adding an id to the hbox can fix
> it.

Comment 10

[Soumya Deb [:Debloper]]

(In reply to comment #9)
> I'd suggest "identity-popup-button-container" as the id, given this rationale:
> 
> > If for any reason, a button is required to be added in that hbox set, it takes
> > relatively complex approach to get it done. Adding an id to the hbox can fix
> > it.

Nice suggestion & I've also given similar thoughts before.
One thing kept me from doing it is, all the child nodes descendant from "identity-popup" are inheriting the 1st part of their parent's id, then appending their own. But giving that ID will break it for the contained button's id. (I'd request taking a look at lines 342-384)

Although I know having "identity-popup-button-container" sounds much better, but this previous convention (albeit not mine) is not very bad anyway. I have no bias, for any id (all I want is a ID, which should be added there).

Let me know; I'll change & resubmit.

Comment 11

[Dão Gottwald [:dao]]

(In reply to comment #10)
> Although I know having "identity-popup-button-container" sounds much better,
> but this previous convention (albeit not mine) is not very bad anyway.

It's not an intended convention and doesn't need to be followed through.

Comment 12

[Soumya Deb [:Debloper]]

Attachment: Changed ID to Dao suggested value

Comment 13

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/37e106e25d9c