There's no arm in landing in stubs so that plugins can safely query this.

Comment on attachment 632314 [details] [diff] [review] Stub This is wrong in several ways. First, there's no point in making any call from the browser to the plugin's NPP_SetValue() to change NPNVcontentsScaleFactor if the browser doesn't yet support changing the scale factor. Second, your change to PluginInstanceChild.cpp isn't the right way to implement this. You'd need to model your code on how we call a plugin's NPP_SetValue() to change NPNVprivateModeBool. So let's just drop your changes to PluginInstanceChild.cpp. Third, the correct way to implement your changes to nsNPAPIPlugin.cpp is as follows: case NPNVcontentsScaleFactor: { // Until bug 674373 is fixed we only support a scale factor of 1.0. *(double*) result = 1.0; return NPERR_NO_ERROR; }

(In reply to Steven Michaud from comment #2) > Comment on attachment 632314 [details] [diff] [review] > Stub > > This is wrong in several ways. > > First, there's no point in making any call from the browser to the > plugin's NPP_SetValue() to change NPNVcontentsScaleFactor if the > browser doesn't yet support changing the scale factor. > And this patch doesn't to this. It lets the plugin query the scale factor. Which we will unconditionally return 1.0 until it is implemented properly. > Second, your change to PluginInstanceChild.cpp isn't the right way to > implement this. You'd need to model your code on how we call a > plugin's NPP_SetValue() to change NPNVprivateModeBool. > There's no need to call SetValue if we don't change the contentscale during the lifetime of the plugins. > So let's just drop your changes to PluginInstanceChild.cpp. > > Third, the correct way to implement your changes to nsNPAPIPlugin.cpp > is as follows: > > case NPNVcontentsScaleFactor: { > // Until bug 674373 is fixed we only support a scale factor of 1.0. > *(double*) result = 1.0; > > return NPERR_NO_ERROR; > } You're right, the return value is incorrect.

PluginInstanceChild::NPN_SetValue() (where you put your changes) is called (from a plugin, indirectly) to *change* a value in the browser, not to *query* a value. But I took another look and it seems PluginInstanceChild.cpp does need changes -- just not the ones you made, and not where you made them. Add the following to XP_MACOSX part of PluginInstanceChild::NPN_GetValue(): case NPNVcontentsScaleFactor: { *((double*)aValue) = 1.0; return NPERR_NO_ERROR; }

(In reply to Steven Michaud from comment #4) You're right. I made the change quickly and it went in the wrong place.

Is there value in keeping this bug open? Sounds like it's the wrong approach, and bug 785667 will take care of making plugins work for HiDPI? --> INVALID/DUPE?

I'll "fix" this bug as part of my work at bug 785667 -- I'll return an appropriate value to plugins that query NPNVcontentsScaleFactor. In fact my current patches already do that. Though, as best I can tell, the plugins that query this value behave the same regardless of the answer they get, or whether or not the query is recognized.

(In reply to Steven Michaud from comment #7) > I'll "fix" this bug as part of my work at bug 785667 -- I'll return an > appropriate value to plugins that query NPNVcontentsScaleFactor. Given that bug 785667 is RESOLVED FIXED, is this one resolved too?

Yes.