Problem/Motivation

Currently, the opacity of textareas that are being resized is set directly in the JS. This means themes cannot override it.

Proposed resolution

Instead of setting the opacity, set a CSS class .dragging. Provide default CSS for this class to match the current styling, which can then be overridden by themes,

#835820-30: Don't set CSS directly in JS, make it themable provides this fix rolled against the current D8 codebase. The CSS in this patch is confirmed to work in:

  • FF3 (Windows)
  • Opera 9 (Windows)
  • IE6
  • FF (OS X)
  • Chrome (OS X)
  • Safari (OS X)

IE CSS is included alongside standard CSS. Documentation at https://developer.mozilla.org/en/CSS/opacity confirms the solution used is correct for IE and modern browsers. See #835820-23: Don't set CSS directly in JS, make it themable.

Remaining tasks

None.

User interface changes

None.

API changes

New CSS class (.dragging). Not sure if that counts. :)

Original report by @alippai

textarea.js sets opacity directly in JS instead of adding CSS class and setting that opacity value in CSS

Added D7 and D6 patches too.

CommentFileSizeAuthor
#30 835820-30-textarea-opacity-with-ie6.patch1.11 KBxjm
#24 835820-19-textarea-opacity-with-ie6_0.patch1.05 KBRobLoach
#19 835820-19-textarea-opacity-with-ie6.patch1.05 KBidflood
#17 835820-16-textarea-opacity-with-ie6-reroll.patch1.04 KBidflood
#10 835820-10-textarea-opaciy-withie6.patch1.31 KBPasqualle
#8 835820-textarea-opaciy-withie6.patch1.36 KBalippai
#1 835820-textarea-opacity.patch1.24 KBDamien Tournoud
opacityd6.patch1.4 KBalippai
opacity.patch1.46 KBalippai
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Priority: Minor » Normal
Status: Active » Needs review
FileSize
835820-textarea-opacity.patch1.24 KB

Makes sense. Rerolled as a proper patch.

alippai’s picture

alippai CreditAttribution: alippai commented
Priority: Normal » Minor
alippai’s picture

alippai CreditAttribution: alippai commented
Priority: Minor » Normal

sry, crossposted

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

This seems like quite a reasonable change....

But can someone clue me in as to where this is used in the core UI, so I'd have a chance at testing to see if it still works as it should (i.e. that it is somewhat transparent while it's being dragged)? I'm not sure what dragging a text area would mean, or where this is being used.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

@jhodgdon: That's the whole textarea being set transparent when the resize handle is used.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

In Firefox 3.x (Windows and Linux), I have verified that this works as intended after the patch, on a Node Add page. Using Firebug, I also made sure that the patched JS was actually being called.

The patched site also works as intended in Opera 9 on Windows.

However, I also tried in in Internet Explorer 6. In IE6, the old code worked correctly (the text area went dim while I was resizing), and this new code doesn't (the text area stays as it was). I am not sure why, or if anyone cares about IE6 (I only keep it around for testing purposes), but it is a regression.

alippai’s picture

alippai CreditAttribution: alippai commented
Assigned: Unassigned » alippai
Status: Needs review » Needs work

In jQuery 1.4:

var style = elem.style || elem, set = value !== undefined;

// IE uses filters for opacity
if ( !jQuery.support.opacity && name === "opacity" ) {
    if ( set ) {
        // IE has trouble with opacity if it does not have layout
        // Force it by setting the zoom level
        style.zoom = 1;

        // Set the alpha filter to set the opacity
        var opacity = parseInt( value, 10 ) + "" === "NaN" ? "" : "alpha(opacity=" + value * 100 + ")";
        var filter = style.filter || jQuery.curCSS( elem, "filter" ) || "";
        style.filter = ralpha.test(filter) ? filter.replace(ralpha, opacity) : opacity;
    }

    return style.filter && style.filter.indexOf("opacity=") >= 0 ?
        (parseFloat( ropacity.exec(style.filter)[1] ) / 100) + "":
        "";
}

In jQuery 1.2.6 (for D6):

if ( msie && name == "opacity" ) {
  if ( set ) {
    // IE has trouble with opacity if it does not have layout
    // Force it by setting the zoom level
   elem.zoom = 1;

   // Set the alpha filter to set the opacity
   elem.filter = (elem.filter || "").replace( /alpha\([^)]*\)/, "" ) +
     (parseInt( value ) + '' == "NaN" ? "" : "alpha(opacity=" + value * 100 + ")");
  }
	
  return elem.filter && elem.filter.indexOf("opacity=") >= 0 ?
    (parseFloat( elem.filter.match(/opacity=([^)]*)/)[1] ) / 100) + '' : "";
}

I have to implement theese in CSS (set the zoom and the alpha filter).

alippai’s picture

alippai CreditAttribution: alippai commented
Status: Needs work » Needs review
FileSize
835820-textarea-opaciy-withie6.patch1.36 KB

Status: Needs review » Needs work

The last submitted patch, 835820-textarea-opaciy-withie6.patch, failed testing.

Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle commented
Status: Needs work » Needs review
FileSize
835820-10-textarea-opaciy-withie6.patch1.31 KB

replaced windows style line ends

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Yeah. Apparently IE uses filter rather than opacity as the CSS (apparently, opacity was not a CSS standard until CSS 3, so browsers rolled their own standard). For reference:
http://www.w3schools.com/Css/css_image_transparency.asp

Anyway, I think it could be useful to put a comment in the CSS about why both of those are needed? And are IE fixes supposed to be in the main system CSS file or in an IE-specific file?

So... Those details aside, I have tested this in FF 3/Win, Opera 9/Win, and IE6, and it now works.

alippai’s picture

alippai CreditAttribution: alippai commented

Who should decide on this? Gabor and webchick?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

#10: 835820-10-textarea-opaciy-withie6.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 835820-10-textarea-opaciy-withie6.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Looks like this needs a reroll, and maybe something to address #11?

idflood’s picture

idflood CreditAttribution: idflood commented
Status: Needs work » Needs review
FileSize
835820-16-textarea-opacity-with-ie6-reroll.patch1.04 KB

simple reroll of patch in #10. The question about seperate css for ie remains. ( only tested with firefox, chrome and safari on osx )

alippai’s picture

alippai CreditAttribution: alippai commented
Status: Needs review » Needs work

http://www.quirksmode.org/css/opacity.html - IMHO we need to test (and maybe extend) functionality for IE8. Tomorrow I'll check it.

idflood’s picture

idflood CreditAttribution: idflood commented
Status: Needs work » Needs review
FileSize
835820-19-textarea-opacity-with-ie6.patch1.05 KB

The css code provided by the patch should be enough, according to https://developer.mozilla.org/en/CSS/opacity .

There was a missing space and I also reordered the attributes to match the given reference.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

I just checked if the IE-specific fixes should be in a separate file or not. I suppose not as I found another IE-specific fix in a 'regular' css file. This is part of modules/toolbar/toolbar.css:

filter: progid:DXImageTransform.Microsoft.Shadow(color=#000000, direction='180', strength='10');
-ms-filter: "progid:DXImageTransform.Microsoft.Shadow(color=#000000, direction='180', strength='10')";

Another thought: is it a good idea to provide both the 'filter' and '-ms-filter' statements as is done here in toolbar.css? I guess this is in there for compatibility reasons.

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented
Status: Needs review » Reviewed & tested by the community

I'd consider this RTBC. Not sure how it impacts the IE issue that was mentioned above though.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Version: 7.x-dev » 8.x-dev

Needs to go to d8 and then d7

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

The IE-specifc cases should be in the same file.

According to the mozille pages: (I trust them :) )
- IE supports also the extended form progid:DXImageTransform.Microsoft.Alpha(Opacity=xx). This is needless, so don't use it.
- IE8 introduced -ms-filter, synonymous with filter. Don't use the -ms- prefix.
- Similar to -moz-opacity, -khtml-opacity is dead as well since early 2004 (release of Safari 1.2).
- Konqueror never had support for -khtml-opacity and uses opacity since version 4.0.
- Don't use -khtml-opacity nor -moz-opacity anymore.

I don't think this needs a comment. Maybe a link to https://developer.mozilla.org/en/CSS/opacity could help but I don't link to put external links into core.

So this sounds good to go :)

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented
Status: Reviewed & tested by the community » Needs review
FileSize
835820-19-textarea-opacity-with-ie6_0.patch1.05 KB

Test bot?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs review » Reviewed & tested by the community

Looks good.

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented

Related: #1138258: Drop textarea.js in favor of CSS3 resize

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Issue tags: +Needs backport to D7

Tagging for backport.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: -Needs backport to D7

#24: 835820-19-textarea-opacity-with-ie6_0.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, 835820-19-textarea-opacity-with-ie6_0.patch, failed testing.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Reviewed & tested by the community
FileSize
835820-30-textarea-opacity-with-ie6.patch1.11 KB

Rerolled against current D8 codebase; no changes. Back to RTBC.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

Added issue summary.

droplet’s picture

droplet CreditAttribution: droplet commented

Is that textarea.js a part of FAPI ? I see all other JS have own comments with the css.

and don't you think .textarea-dragging is more better.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

Is that textarea.js a part of FAPI ? I see all other JS have own comments with the css.

Could you clarify?

I agree that .textarea-dragging is a more specific and clear class name. If anyone who participated in the issue earlier agrees, let's reroll it with that name. Should still be RTBC either way.

droplet’s picture

droplet CreditAttribution: droplet commented

It should be something like:

 /**
   * Textarea behavior.
   *
   * @see textarea.js
   */
.foobar { xxxxx }

http://drupalcode.org/project/drupal.git/blob/refs/heads/8.x:/modules/sy...

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Component: other » CSS
Status: Reviewed & tested by the community » Needs work

@droplet is right. Also changing the component to make the right people aware of this issue.

Lastly, I'm not sure whether this can be backported. The purpose of system.theme.css is to be overridden by themes. (but putting that CSS into system.theme.css is valid)

droplet’s picture

droplet CreditAttribution: droplet commented
Version: 8.x-dev » 7.x-dev
Issue tags: -Needs backport to D7

D8 dropped: #1138258: Drop textarea.js in favor of CSS3 resize

droplet’s picture

droplet CreditAttribution: droplet commented
Issue summary: View changes

Updated issue summary.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Assigned: alippai » Unassigned
Issue summary: View changes
brahmjeet789’s picture

brahmjeet789 CreditAttribution: brahmjeet789 commented

I think @droplet is right.that the issue name should be interrelated with the issue so that people should awre of the type of issue that is what is the issue?

R13ose’s picture

R13ose CreditAttribution: R13ose commented
Issue tags: +browser compatibility