Opened 10 years ago

Closed 10 years ago

#10466 closed defect (fixed)

[patch][ccla][regression][minor]: Fontchoice plugin doesn't allow you to set the plugin state to the same value after move

Reported by: Jared Jurkiewicz Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.4.1
Component: Editor Version: 1.4.0b
Keywords: Cc: bill, liucougar, Douglas Hays
Blocked By: Blocking:

Description (last modified by Jared Jurkiewicz)

This is a problem that occurs when the following sequence happens:

Given the following html:

<p>Paragraph 1</p>
<p>Paragraph 2</p>

1.) You click in Paragraph 2. Format dropdown updates to 'Paragraph' (Correct)
2.) Change it to Heading
3.) Click in to Paragraph 1. Format dropdown updates back to 'Paragraph' (Correct)
4.) Try to change it to heading. It will not change.

This occurs because the updateState function makes use of the 'priorityChange' parameter of attr for FilteringSelect?. This was needed because FilteringSelect?'s onChange operates asynchronously. When update state changes the dropdown, you do not want an 'onChange' to happen, because onChange is attached to work as a user change vent and when it happens, the formatblock operation is called. For state update,s you don't want to actually do an operation. Since FilteringSelect? is async, you can't just 'ignore' an event easily (since you don't know precisely when the state is going to fire). So that priorityChange param is very useful here.

The problem comes in, in that priorityChange false doesn't reset the last internal state value. So even though in step 3-4 it updatestate changes to 'Paragrah', internally it still things the state is 'Heading'. So when we try to set to heading, it thinks that is the current state ... and ignores the event, no onchange fies, and no formatblock is called.

Douglas Hays said this is expected behavior of using priorityChange, unfortunately. He did give me a workaround for the issue, though, and I'll provide a patch plus unit test case for it shortly.

Targetting for 1.4.1, as this is a really annoying behavior and is a regression from 1.3.2. (That all said, the plugin still works far better than it didn in 1.3.2, I fixed tons of cross-browser behavioral issues).

Attachments (2)

FontChoice.patch (656 bytes) - added by Jared Jurkiewicz 10 years ago.
Small patch that fixes the issue
FontChoice_ut.patch (8.5 KB) - added by Jared Jurkiewicz 10 years ago.
Updated UT, fixed some tab/spaces issues.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by Jared Jurkiewicz

Description: modified (diff)

comment:2 Changed 10 years ago by Jared Jurkiewicz

Cc: bill liucougar Douglas Hays added

comment:3 Changed 10 years ago by Jared Jurkiewicz

Status: newassigned

Changed 10 years ago by Jared Jurkiewicz

Attachment: FontChoice.patch added

Small patch that fixes the issue

comment:4 Changed 10 years ago by Jared Jurkiewicz

Summary: [regression][minor]: Fontchoice plugin doesn't allow you to set the plugin state to the same value after move[patch][ccla][regression][minor]: Fontchoice plugin doesn't allow you to set the plugin state to the same value after move

Changed 10 years ago by Jared Jurkiewicz

Attachment: FontChoice_ut.patch added

Updated UT, fixed some tab/spaces issues.

comment:5 Changed 10 years ago by Jared Jurkiewicz

(In [20988]) Checking in fix for FontChoice? + UT. refs #10466

comment:6 Changed 10 years ago by Jared Jurkiewicz

Resolution: fixed
Status: assignedclosed

(In [21004]) Committing fix + ut update for 1.4.1. fixes #10466

Note: See TracTickets for help on using tickets.