Hi folks, I'm currently investigating how to get rid of direct mdb references out of the frontend code. One point is abCommon.js -> isCommandEnabled(): It checks the special URIs (personal and collected) to decide whether the address book can be deleted (for the context menu button). > // If it's one of these special ABs, return false to disable deletion. > if (selectedDirURI == kPersonalAddressbookURI || > selectedDirURI == kCollectedAddressbookURI) > return true; For mailing lists, it checks the readOnly attribute: > // If the directory is a mailing list, and it is read-only, > // return false to disable deletion. > if (selectedDir.isMailList && selectedDir.readOnly) > return false; Why not letting the special dirs be readonly and always check for that attribute (not just for mailing lists) ? If there's some other meaning to readOnly, which might conflict here, why not introducing a new attribute (eg. "canDelete") which does the proper checks ? That way, the frontend doesn't need any special magic for this anymore (having that encapsulated within nsIDirectory). OTOH, if we need the special types anyways (eg. some other place uses it for sorting), we could introduce some "type" attribute or individual flags. --mtx

On 8/17/2017 3:20 AM, Enrico Weigelt, metux IT consult via Maildev wrote: > Hi folks, > > > I'm currently investigating how to get rid of direct mdb references out > of the frontend code. One point is abCommon.js -> isCommandEnabled(): > > It checks the special URIs (personal and collected) to decide whether > the address book can be deleted (for the context menu button). > > > // If it's one of these special ABs, return false to disable deletion. > > if (selectedDirURI == kPersonalAddressbookURI || > > selectedDirURI == kCollectedAddressbookURI) > > return true; > > For mailing lists, it checks the readOnly attribute: > > > // If the directory is a mailing list, and it is read-only, > > // return false to disable deletion. > > if (selectedDir.isMailList && selectedDir.readOnly) > > return false; > > Why not letting the special dirs be readonly and always check for that > attribute (not just for mailing lists) ? > > If there's some other meaning to readOnly, which might conflict here, > why not introducing a new attribute (eg. "canDelete") which does the > proper checks ? That way, the frontend doesn't need any special magic > for this anymore (having that encapsulated within nsIDirectory). > Generally yes the correct thing to do is to add the canDelete attribute to the nsIAbDirectory, and use that instead of specific checks of ab directory types. Those types of checks are what make it hard to implement new types (like Carddav or EWS) that are not part of the core. I've complained about that issue in mail code as well, where we have (if type == "imap" then ...). At this point, we should really take this discussion off of maildev and onto a bug. We are getting into too much detail for maildev, which is really meant for larger issues of policy and planning rather than discussions of specific code changes. :rkent