Closed Bug 667952 Opened 9 years ago Closed 9 years ago

Fold imageContextOverlay.xul into context menu code

Categories

(SeaMonkey :: Passwords & Permissions, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.4

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Fold in imageContextOverlay.xul (obsolete) — Splinter Review
At the moment, for historical reasons, we have some permissions overlays such as imageContextOverlay.xul. This could be folded into the context menu code and hopefully help tidy up the code at the same time.

This patch:
* Moves xul code into contentAreaContextOverlay.xul and mailWindowOverlay.xul files.
* Moves js code into nsContextMenu.js with some simplification.
* Removes the xul file and jar.mn entries.

Just wondering if the image context menu item orders should match between contentAreaContextOverlay.xul and mailWindowOverlay.xul
Attachment #542516 - Flags: review?(neil)
Comment on attachment 542516 [details] [diff] [review]
Fold in imageContextOverlay.xul

>+      var uri = Services.io.newURI(this.mediaURL, null, null);
>+      if (uri & uri.host) {
newURI won't fail, because we got mediaURL from the spec of another URL. But .host can throw; I think maybe a data: image can do that, for instance. (And the single & was wrong, too!)

>+  toggleImageBlocking: function CM_toggleImageBlocking(aBlock) {
[Bah, those funky function names...]

>-    <menuitem id="context-blockimage"
>-              oncommand="cookieContextMenu.toggleImageBlocking(true);"
>-              insertafter="context-viewimage"/>
>-    <menuitem id="context-unblockimage"
>-              oncommand="cookieContextMenu.toggleImageBlocking(false);"
>-              insertafter="context-viewimage"/>
[I wonder whether technically this ends up inserting the two menuitems in reverse order]
Attachment #542516 - Flags: review?(neil) → review-
Comment on attachment 542516 [details] [diff] [review]
Fold in imageContextOverlay.xul

>    content/communicator/permissions/cookieViewer.js                 (permissions/cookieViewer.js)
>    content/communicator/permissions/cookieViewer.xul                (permissions/cookieViewer.xul)
>-   content/communicator/permissions/imageContextOverlay.xul         (permissions/imageContextOverlay.xul)
>    content/communicator/permissions/permissionsManager.js           (permissions/permissionsManager.js)
>    content/communicator/permissions/permissionsManager.xul          (permissions/permissionsManager.xul)
>    content/communicator/permissions/permissionsNavigatorOverlay.xul (permissions/permissionsNavigatorOverlay.xul)
>    content/communicator/permissions/treeUtils.js                    (permissions/treeUtils.js)
BTW, didn't Data Manager replace this?
While the Data Manager still has significant UX problems, I'd like the old managers to be still accessible and in working condition.
Changes since last version:
* Removed funky function name.
* Better detection of missing uri.host.

The old way would have inserted in reverse order, but as one is only ever shown, order does not really matter. Old manager UI needs to stay for the moment, but that is another bug.
Attachment #542516 - Attachment is obsolete: true
Attachment #543298 - Flags: review?(neil)
Comment on attachment 543298 [details] [diff] [review]
Fold in imageContextOverlay.xul with less funky stuff [Checked in: Comment 6]

>+      var uri = Services.io.newURI(this.mediaURL, null, null);
>+      if (uri && uri instanceof Components.interfaces.nsIURL && uri.host) {
Nit: don't need the uri && because
a) newURI never returns null
b) instanceof always returns false for null anyway
Attachment #543298 - Flags: review?(neil) → review+
Comment on attachment 543298 [details] [diff] [review]
Fold in imageContextOverlay.xul with less funky stuff [Checked in: Comment 6]

http://hg.mozilla.org/comm-central/rev/9f8456bb68f6
Attachment #543298 - Attachment description: Fold in imageContextOverlay.xul with less funky stuff → Fold in imageContextOverlay.xul with less funky stuff [Checked in: Comment 6]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.4
You need to log in before you can comment on or make changes to this bug.