nus-oss-test / testrepo4

TEAMMATES system is online at
http://teammatesv4.appspot.com
0 stars 0 forks source link

instructorFeedbackComment: Comment feature does not work on Firefox 27/28 #1773

Closed damithc closed 10 years ago

damithc commented 10 years ago

From dam...@gmail.com on March 27, 2014 20:03:51

When I click on the comment box, nothing happens. Works on FF12 (the one used for testing), latest chrome, and IE. Are we using a non compatible function?

Original issue: http://code.google.com/p/teammatespes/issues/detail?id=1758

damithc commented 10 years ago

From dam...@gmail.com on March 27, 2014 05:10:59

WeiLing, could you have a look at this?

Owner: lwl1991
Cc: arnold.k...@gmail.com

damithc commented 10 years ago

From lwl1991 on March 27, 2014 05:21:14

Sure. Will check it out.

damithc commented 10 years ago

From lwl1991 on March 27, 2014 06:36:13

Somehow it does not work on both FF 12 and 28 for me (after i accidentally upgraded FF12 to 28, and downgraded it again. Not sure if it worked before the upgrade.)

The javascript/jQuery attached to the button seems to be working, since i managed to call it through the console(and it works).

It seems like this might be a issue with firefox when disabled inputs are used. According to this answer, firefox does not handle/fire any events from disabled elements, and therefore the javascript function is not called at all. http://stackoverflow.com/questions/3100319/event-on-a-disabled-input Perhaps simply enabling the input would solve this. The focus will still change properly later.(But it won't look grayed out) Otherwise, we could try overlaying another element on top of the disabled input to trigger the click event, like what is done in the SO answer.

damithc commented 10 years ago

From dam...@gmail.com on March 27, 2014 18:38:29

We can try 'not disabling' it, as long as it does not degrade the usability too much. Seems that is the easier option.

damithc commented 10 years ago

From lwl1991 on March 27, 2014 18:46:10

I am almost done with the overlapping element solution. Visually it won't make a difference. Will submit the patch once I check all tests pass.

Status: Started

damithc commented 10 years ago

From dam...@gmail.com on March 27, 2014 19:12:08

WeiLin, the other option is to use a normal 'Add comment' link. It's cleaner too. We can replace it with an icon later. We can go with the overlay solution for now, though.

damithc commented 10 years ago

From lwl1991 on March 27, 2014 20:11:24

Uploaded the overlay version. https://codereview.appspot.com/81690043/ Perhaps we could standardize all comments in TEAMMATES to look the same (and use the same code) at a later stage? Right now they all seem to be implemented separately in their various pages.

Status: ReadyForReview

damithc commented 10 years ago

From dam...@gmail.com on March 28, 2014 05:23:53

This issue was updated by revision 0d521c4e3e69 .

Status: Delivered

damithc commented 10 years ago

From dam...@gmail.com on March 29, 2014 03:03:27

Status: Deployed
Labels: Milestone-V4.94