Open
Bug 757230
Opened 12 years ago
Updated 8 years ago
When using add button for permissions in Data Manager set a displayHost
Categories
(SeaMonkey :: Passwords & Permissions, defect)
SeaMonkey
Passwords & Permissions
Tracking
(Not tracked)
ASSIGNED
People
(Reporter: iannbugzilla, Assigned: iannbugzilla)
Details
Attachments
(1 file, 1 obsolete file)
2.97 KB,
patch
|
kairo
:
review+
|
Details | Diff | Splinter Review |
For other parts of Data Manager that deal with permissions the displayHost is set but not for when using the add button.
Attachment #625779 -
Flags: review?(kairo)
Comment 1•12 years ago
|
||
Comment on attachment 625779 [details] [diff] [review] Set displayHost Adding the displayHost is OK and good, thanks, but the change to rawHost is inconsistent with what is being done in other places in the Data Manager. It should still have an eventual leading dot stripped. host is where that's not stripped. Also, the places in permissions_reactToChange where entries are added also don't set a displayHost. And is rawHost actually still used somewhere now or do we only set it?
Attachment #625779 -
Flags: review?(kairo) → review-
I cannot see anywhere that uses the rawHost attribute within permissions but you probably know the code better than me.
Attachment #625779 -
Attachment is obsolete: true
Attachment #629739 -
Flags: review?(kairo)
Comment 3•12 years ago
|
||
Comment on attachment 629739 [details] [diff] [review] Use displayHost in more places Review of attachment 629739 [details] [diff] [review]: ----------------------------------------------------------------- The change itself is good, but I still worry about rawHost probably being unused. (In reply to Ian Neal from comment #2) > I cannot see anywhere that uses the rawHost attribute within permissions but > you probably know the code better than me. If you don't try removing it and check that all tests still run well (including some rough manual testing, but mainly the automated ones that should catch stuff), then please at least file a followup. It's been a while since I worked in this code and while I know it was used before, it might have been completely obsoleted by displayHost nowadays.
Attachment #629739 -
Flags: review?(kairo) → review+
Comment 4•8 years ago
|
||
I think due to the changes in the Permission api and the changes in Bug 1188348 the patch is invalid. But I didn't set a proper display IDN yet because I was unable to find one which would not relocate me to an ascii domain so not sure if it should be kept open.
You need to log in
before you can comment on or make changes to this bug.
Description
•