94749 - Simplify image map code in Image properties dialog

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[Charles Manske]

The dialog JS currently has 2 global variables: "imageMap" and "globalMap" 
that must be kept in synch. It is not clear why there are two -- it seems 
unnecessary. If it is to maintain a copy to allow cancelling after editing the 
image map, this copy should be created by the image map dialog(s), not the
Image Properties dialog.

Comment 1

[Syd Logan]

moving out to 0.9.6

Comment 2

[Syd Logan]

spam composer change

Comment 3

[Charles Manske]

Done as part of general dialog cleanup: bug 93732.

*** This bug has been marked as a duplicate of 93732 ***

Comment 4

[James Baxter]

Verified Dup

Comment 5

[Charles Manske]

Too many changes already in bug 93732. All of those changes are common to 
all dialogs. Separating this out, since it applies only to Image dialog.

Comment 6

[Charles Manske]

Attachment: Fix: Eliminate double imageMap object

Comment 7

[Charles Manske]

Attachment: Updated patch: Don't remove Image map until onOk(), so Cancel results in untouched document.

Comment 8

[Kathleen :Brade]

Comment on attachment 53276 [details] [diff] [review]
Updated patch: Don't remove Image map until onOk(), so Cancel results in untouched document.

r=brade

Comment 9

[kinmoz]

Comment on attachment 53276 [details] [diff] [review]
Updated patch: Don't remove Image map until onOk(), so Cancel results in untouched document.

Are you sure you are calling editorShell.EndBatchChanges() everywhere you should 

be? From reading the diffs it looks like you are doing something like this:

    editorShell.EndBatchChanges();

    ...

    if (ValidateData())
    {
      ...
      editorShell.EndBatchChanges();
      SaveWindowLocation();
      return true;
    }

    return false;


Which means that if Validate Data() was ever false,
you'd have an unbalanced BeginBatchChanges() which
would leave the editor in
state where nothing would render or reflow.

Comment 10

[kinmoz]

Bah, typo in my comment above ... the first occurrence of EndBatchChanges(), in
the code snippet above should be a BeginBatchChanges().

Comment 11

[kinmoz]

Comment on attachment 53276 [details] [diff] [review]
Updated patch: Don't remove Image map until onOk(), so Cancel results in untouched document.

Rather than hold you up for another round of reviews ...
assuming ValidateData() doesn't require batching,
if you simply move the BeginBatchChanges() under the ValidateData()
'if' statement ... consider this sr=[e-mail]

Comment 12

[Charles Manske]

Right, I simply moved beginBatchChanges like this:

  if (ValidateData())
  {
    editorShell.BeginBatchChanges();

... (no returns in here)

     editorShell.EndBatchChanges();

    SaveWindowLocation();
    return true;
  }

Comment 13

[Charles Manske]

checked in.

Comment 14

[sujay]

Charley, can you verify this one? thanks.

Comment 15

[Charles Manske]

verified