The current versions of abNewCardDialog.xul and abEditCardDialog.xul are very similar apart from some small formatting differences and a couple of different dialog ids. Following on from bug 350992 I'd like to propose that we make these xul files common between SM & TB as well. At a minimum we should make the dialog ids the same to help extension writers. One of the things I'd like Scott's & Neil's agreement on before we do this is the license block. TB pre-processes these out, whereas SM keeps them as comments. Comments please? (btw I plan to suggest we do some of the js files but that'll be under different bugs)
I'm not a fan of preprocessing. Apparently it's becoming to be a more and more significant portion of build time. It also makes it difficult to open the source file in an xml-compliant editor, although there the workaround is to change the -s on the middle lines to #s something like this: <!-- BEGIN LICENCE STUFF - # Waffle, Blah (would have read - Waffle, Blah) - END LICENCE STUFF -->
given: a) AB coder resources are scarce, and b) the whole AB area seems headed for needed improvements convergence will be a very "good thing".
Created attachment 281019 [details] [diff] [review] Re-merge card dialogs This patch re-merges the new & edit card dialogs so that SM & TB share the same code. The most significant difference is the id of the dialog. The patch changes the TB id from "abcardDialog" to "abcardWindow". The SM one stays at "abCardWindow". Although these are dialogs, I chose to go with abCardWindow as then the window icons (http://mxr.mozilla.org/seamonkey/find?string=abcardWindow) will work for both SM and TB - currently they work for only SM. If we prefer to go with abcardDialog, then we could rename the window icons. This will possibly break some TB extensions, but considering the recent address book changes anyway, we've possibly broken them already. The plus side is that it'll be easier for extension developers to hook into the dialogs if they are writing an extension aimed at SM and TB. The other main difference is that TB currently removes the license header, whereas the new version does not. We can always discuss this.
Comment on attachment 281019 [details] [diff] [review] Re-merge card dialogs I don't think we're making claims about compatibility with 2.0 extensions so this change should be ok.
Attachment #281019 - Flags: superreview?(mscott) → superreview+
Patch checked in -> fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
No need for blocking request now.
You need to log in before you can comment on or make changes to this bug.